feat(feature-flags): implement early exit functionality for feature flag conditions#57321
Open
gustavohstrassburger wants to merge 26 commits into
Open
feat(feature-flags): implement early exit functionality for feature flag conditions#57321gustavohstrassburger wants to merge 26 commits into
gustavohstrassburger wants to merge 26 commits into
Conversation
…lag conditions This adds a new early exit feature that allows feature flag conditions to stop evaluation when users match conditions but are excluded by rollout percentage, rather than continuing to evaluate subsequent conditions. Changes: - Add early_exit field to FeatureFlagGroupType model and API schema - Implement early exit evaluation logic in Rust flags engine - Add UI checkbox in V2 feature flag editor for early exit configuration - Display early exit status in read-only feature flag details view - Update TypeScript types and logic to support early exit functionality The early exit behavior ensures deterministic flag evaluation while providing more control over flag condition processing order. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
Size Change: +5.17 kB (+0.01%) Total Size: 80.8 MB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
gustavohstrassburger
force-pushed
the
gustavo/early-exit-flags
branch
from
4478911 to
7ae89f9
Compare
Contributor
Query snapshots: Backend query snapshots updatedChanges: 4 snapshots (4 modified, 0 added, 0 deleted) What this means:
Next steps:
|
Contributor
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
- Add missing earlyExit parameter to updateConditionSet type definition - Fix labelInline prop to use explicit boolean value 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![hex-security-app[bot]](https://avatars.githubusercontent.com/in/2665233?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an “early exit” option to feature flag condition sets so that when a user matches a condition’s targeting but falls outside its rollout percentage, evaluation can stop immediately (returning false) instead of continuing to later condition sets.
Changes:
- Extend feature flag condition-set schema/types (
early_exit) across backend OpenAPI docs, MCP tool schemas, Rust flag engine models, and frontend TypeScript types. - Implement early-exit evaluation behavior in the Rust feature flag matcher.
- Add UI controls and read-only display for configuring/viewing early exit in the feature flag editor.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| services/mcp/tests/unit/snapshots/tool-schemas/common/update-feature-flag.json | Updates MCP tool JSON schema snapshot to include early_exit on condition groups. |
| services/mcp/tests/unit/snapshots/tool-schemas/common/create-feature-flag.json | Updates MCP tool JSON schema snapshot to include early_exit on condition groups. |
| services/mcp/tests/unit/snapshots/tool-schemas/common/survey-create.json | Updates survey tool schema snapshot to include early_exit on targeting flag filters groups. |
| services/mcp/tests/unit/snapshots/tool-schemas/common/survey-update.json | Updates survey tool schema snapshot to include early_exit on targeting flag filters groups. |
| services/mcp/src/generated/surveys/api.ts | Adds early_exit to Zod schemas for survey create/partial update targeting flag filters. |
| services/mcp/src/generated/feature_flags/api.ts | Adds early_exit to Zod schemas for feature flag create/partial update filters. |
| services/mcp/src/api/generated.ts | Updates generated API types to include early_exit in schemas. |
| rust/feature-flags/src/flags/flag_models.rs | Adds early_exit to Rust FlagPropertyGroup model + mocks. |
| rust/feature-flags/src/flags/flag_matching.rs | Implements early-exit evaluation logic in Rust matcher. |
| rust/feature-flags/src/flags/test_flag_matching.rs | Updates existing Rust tests/fixtures to include early_exit field. |
| rust/feature-flags/src/flags/flag_property_group.rs | Updates expected structs in tests to include early_exit. |
| rust/feature-flags/src/flags/flag_definitions_cache.rs | Updates cache tests/fixtures to include early_exit. |
| posthog/api/documentation.py | Documents early_exit in OpenAPI serializer schema for condition groups. |
| frontend/src/types.ts | Extends FeatureFlagGroupType TS interface with early_exit. |
| frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.ts | Adds state/update plumbing for editing early_exit per condition set. |
| frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsReadonly.tsx | Displays early-exit status in read-only condition set cards. |
| frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsCollapsible.tsx | Adds checkbox control to configure early exit in the editor UI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add boolean type validation for early_exit field in feature flag groups to prevent Rust serde failures and cache poisoning. Follows the existing pattern used for variant and aggregation_group_type_index validation. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
Query snapshots: Backend query snapshots updatedChanges: 1 snapshots (1 modified, 0 added, 0 deleted) What this means:
Next steps:
|
…/posthog into gustavo/early-exit-flags
- Simplify early exit logic in Rust by removing redundant property checks - Add comprehensive tests for early exit behavior and disabled early exit - Fix TypeScript types to use boolean instead of boolean | null - Update frontend logic to handle early exit parameter correctly
dmarticus
reviewed
May 1, 2026
Contributor
dmarticus
left a comment
dmarticus
left a comment
There was a problem hiding this comment.
IMO this should be wrapped in an early access feature so that you can get some feedback on the experience when rolling it out.
Needs docs, too, but I would say roll it behind a flag -> write docs -> roll it out further.
| checked | ||
| ) | ||
| }} | ||
| label="Stop evaluation if excluded by rollout" |
Contributor
There was a problem hiding this comment.
can we call this e.g. "short-circuit evaluation on false"? I get what you're saying here, but I'm not sure about the terminology.
@ksvat you've felt passionately about this subject – what do you think about this? UI looks like
Contributor
There was a problem hiding this comment.
sorry finally remembered this tag @dmarticus
tbh maybe just my brain being silly but this copy is sort of confusing imo
I'd lean more toward something like "Stop evaluation at first matching group" "Evaluate at first matching group" "Use first matching group to evaluate" or something like that, since its true for both inclusion and exclusion, and its more about the MATCH -> final outcome, than about the exclusion having unique behavior idk
Tooltip could then be something similarly worded like:
When this setting is enabled, conditions are evaluated in order. The first matching condition set determines the result — later conditions are skipped. When this settign is disabled, all conditions are evaluated, and a pass on any condition is a pass, regardless of rollout exclusions on other condition groups.`` `` Optionally include a simple example idk
| )} | ||
|
|
||
| <div className="mt-3 flex items-center gap-2"> | ||
| <LemonCheckbox |
Contributor
There was a problem hiding this comment.
on one hand, I see the value of doing this on a condition-by condition basis so that people have the flexibility to build flags with whatever rules they want.
on the other hand, I also think it could be cool to do this on a flag-level, so that the user doesn't have to think about the conditions and just make flags that short-circuit by default.
dmarticus
reviewed
May 1, 2026
Contributor
There was a problem hiding this comment.
A few additional things to consider on top of the bot feedback and my earlier comments:
SDK local-evaluation divergence
The Rust engine handles early_exit, but PostHog's server-side SDKs (posthog-python, posthog-node, posthog-go, posthog-ruby, posthog-php, posthog-dotnet, posthog-elixir) all perform local evaluation of release conditions when a customer fetches flag definitions and calls feature_enabled() with properties. None of them currently know about early_exit — posthog-python/posthog/feature_flags.py::is_condition_match is the equivalent of is_condition_match in flag_matching.rs and it has no notion of short-circuiting on rollout exclusion.
That means a customer using local eval will get the opposite result from /flags/: local eval continues to the next condition, the Rust service short-circuits. That directly undermines the stated goal of more deterministic flag behavior.
Suggestions:
- Land follow-up PRs to each server SDK's local-eval to honor
early_exit, ideally before announcing the feature. - Until SDKs catch up, either gate the UI behind a feature flag, or surface a banner/help text warning that local eval doesn't yet honor this setting.
- Mention SDK version requirements in the field's
help_textso API consumers know.
Missing Python tests for the new validation
posthog/api/feature_flag.py:917-922 adds the early_exit type check (good fix for the hex-security finding), but no Python test in this PR exercises that branch. Add a parameterized test in posthog/api/test/test_feature_flag.py covering early_exit=true, false, missing, and rejected non-boolean values (1, \"true\", etc.).
Unrelated snapshot churn
posthog/hogql_queries/web_analytics/test/__snapshots__/test_web_stats_table.ambr has 78/-36 lines of changes that have nothing to do with feature flags. Looks like a local snapshot regenerated from a stale base. Worth reverting before merge so it doesn't mask the real snapshot owner's diff next time and pollute blame.
PR description nit
Add
early_exitfield toFeatureFlagGroupTypemodel
There's no Django model field — it's a key inside the existing filters JSON blob. No migration was needed, and that's the right call. Worth correcting the description so future archaeology isn't misled into looking for a column.
Activity logging coverage
Worth verifying that toggling early_exit produces a useful entry in the flag's activity log. Changes to keys inside the filters JSON blob often get summarized as a generic "filters changed" diff. If the activity describer special-cases rollout / variant changes per condition, consider extending it for early_exit so reviewers can see who toggled it and on which condition.
Test nit
The second Rust test's trailing comment (// The test should match the second condition since the first has 0% rollout) reads like a TODO. Either assert condition_index == Some(1) with reason == ConditionMatch, or drop the comment.
haacked
reviewed
May 6, 2026
Contributor
haacked
left a comment
haacked
left a comment
There was a problem hiding this comment.
Nice, focused feature. The core matcher change is clean and the bot reviews mostly turned out to be false positives (Greptile/Copilot misread the early-exit branch as re-evaluating properties; @Hex-Security cache-poisoning concern is closed by the new isinstance validation). Inline suggestions below. No blockers from me, though strong agreement with @dmarticus that the SDK local-eval divergence is the headline risk and worth either gating behind an early-access flag or coordinating SDK rollouts before this lands. Also: please revert the unrelated test_web_stats_table.ambr snapshot churn before merging.
Contributor
Query snapshots: Backend query snapshots updatedChanges: 4 snapshots (4 modified, 0 added, 0 deleted) What this means:
Next steps:
|
gustavohstrassburger
force-pushed
the
gustavo/early-exit-flags
branch
from
c4cac78 to
2e2b015
Compare
Contributor
Query snapshots: Backend query snapshots updatedChanges: 4 snapshots (4 modified, 0 added, 0 deleted) What this means:
Next steps:
|
Contributor
Query snapshots: Backend query snapshots updatedChanges: 2 snapshots (2 modified, 0 added, 0 deleted) What this means:
Next steps:
|
…flags # Conflicts: # frontend/src/types.ts
…ut local evaluation
…ustavo/early-exit-flags
…ed and guard undefined runtime
👥 Auto-assigned reviewersSkipped a review request for |
Contributor
|
Reviews (2): Last reviewed commit: "Merge branch 'master' into gustavo/early..." | Re-trigger Greptile |
This was referenced May 29, 2026
Draft
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Contributor
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
haacked
approved these changes
May 29, 2026
Contributor
haacked
left a comment
haacked
left a comment
There was a problem hiding this comment.
A few suggestions to consider, but nothing majorly blocking.
| </div> | ||
| {releaseFilters.early_exit && | ||
| evaluationRuntime !== undefined && | ||
| evaluationRuntime !== FeatureFlagEvaluationRuntime.SERVER && ( |
Contributor
There was a problem hiding this comment.
suggestion: This banner gates on evaluationRuntime, but that field is a targeting hint, not a signal of how the flag is actually evaluated, so the warning can't really be accurate. early_exit is honored wherever evaluation goes through the /flags endpoint (the Rust matcher), which is every client-side SDK and any server SDK that calls /flags. It's ignored only by server-side SDKs configured for local evaluation, since those evaluate all condition groups in-process. Whether a given consumer has local eval turned on isn't something the flag's runtime setting tells you.
So the current gate is backwards on the one case it can be sure about: CLIENT-only flags never local-eval, so they always honor early_exit, yet they're the ones that get the warning. SERVER/ALL flags are where local eval is even possible, and those are exactly where the banner stays silent. The banner text ("Client-side and local evaluation SDKs evaluate all condition groups") has the same flip, since client-side SDKs do honor early_exit.
Since you can't know local-eval status from here, the honest version is a conditional caveat rather than a flat "not supported." Show it wherever local eval is possible (everything except CLIENT-only) and word it as a maybe:
{releaseFilters.early_exit && evaluationRuntime !== FeatureFlagEvaluationRuntime.CLIENT && (
<LemonBanner type="warning" className="mt-1">
<b>Not applied with local evaluation.</b> If you evaluate this flag with a server-side SDK
using local evaluation, early exit won't take effect, those SDKs evaluate all condition
groups. It does take effect when the flag is evaluated through the /flags endpoint.
</LemonBanner>
)}
One thing I didn't confirm: whether /local_evaluation filters flags by runtime. If it returns CLIENT flags too, then even a CLIENT flag could be locally evaluated and the caveat should show for all runtimes (drop the gate entirely).
| if p.get("operator") in ("regex", "not_regex") and isinstance(p.get("value"), str): | ||
| existing_patterns.add(p["value"]) | ||
|
|
||
| early_exit = filters.get("early_exit") |
Contributor
There was a problem hiding this comment.
question: The feature-flag-early-exit gate is UI-only. This validator type-checks early_exit but doesn't check the feature flag or team eligibility, so the public REST API, the MCP create/update tools, and the terraform provider can all persist early_exit: true regardless of whether the flag is enabled for the team. Combined with the local-eval divergence, that means a customer or an MCP agent could turn this on before the server-side SDKs honor it. Is leaving the API ungated intentional (UI-gated rollout only), or should validate_filters reject a truthy early_exit unless the team has feature-flag-early-exit enabled?
|
|
||
| return { ...state, groups } | ||
| }, | ||
| setEarlyExit: (state, { earlyExit }) => { |
Contributor
There was a problem hiding this comment.
suggestion: setEarlyExit is the only thing that writes early_exit. Worth a unit test to make sure nothing breaks it in the future?
| format="json", | ||
| ) | ||
| self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) | ||
| self.assertIn(expected_type, response.json()["detail"]) |
Contributor
There was a problem hiding this comment.
suggestion: assertIn(expected_type, response.json()["detail"]) is a loose check. The type names are substrings of common words ("int" in "integer", "str" in "string"), so if some other validator rejected the payload first with a message that happened to contain the substring, this would pass without the early_exit branch ever running. The payloads only have early_exit wrong today, but the assertion doesn't pin that. Add the branch-specific text:
Suggested change
| self.assertIn(expected_type, response.json()["detail"]) | |
| self.assertIn("early_exit must be a boolean", response.json()["detail"]) | |
| self.assertIn(expected_type, response.json()["detail"]) |
Contributor
|
I like that this is gated behind a feature flag. But what's the rollout strategy once we've validated this approach? This would be a breaking change for folks. Will we want to make this the new default behavior and existing customers will have the legacy behavior until they opt-in? Or do you have other plans? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When feature flag conditions are evaluated sequentially, users who match conditions but are excluded by rollout percentage continue through subsequent condition evaluation. This can lead to unexpected behavior where users might match a later condition instead of receiving a consistent "false" result.
Changes
early_exitfield toFeatureFlagGroupTypemodel and API schemaThe early exit feature allows flag conditions to stop evaluation immediately when users match targeting criteria but are excluded by rollout percentage, providing more deterministic flag behavior.
How did you test this code?
🤖 Generated with Claude Code