Repository navigation
feat(flags): enforce filters validation one rule at a time - #95226
Merged
Merged
Conversation
Replaces the all-or-nothing enforcement switch with a set of rule ids to reject on. A rule that is not listed keeps logging, including when another rule in the same request rejects, so a rule can go live once its own violations reach zero. Unset matches the old off and "*" matches the old on.
|
😎 Merged successfully - details. |
Contributor
🤖 CI report
|
Contributor
Prompt To Fix All With AI### Issue 1
posthog/settings/feature_flags.py:135-137
**Setting documentation missing**
This adds the `FEATURE_FLAG_FILTERS_ENFORCED_RULES` configuration option without updating the matching documentation under `docs/`. The repository requires configuration changes and their documentation to ship in the same PR, so this must be addressed before merging.
### Issue 2
posthog/settings/feature_flags.py:135-137
**Deployment configuration missing**
This introduces the `FEATURE_FLAG_FILTERS_ENFORCED_RULES` environment variable. The repository requires new environment variables to also be added to `posthog/charts` and supplied through `posthog/secrets`; without that deployment setup, this staged enforcement cannot be activated in production. This requirement must be satisfied before merging.
### Issue 3
products/feature_flags/backend/api/feature_flag.py:1681
**Structural filtering lacks coverage**
This branch independently filters structural errors before rejecting, but the new partial and mixed-enforcement tests intentionally use structurally valid cross-field violations. Add a case with two structural violations and only one configured rule, then assert that the response contains only that rule. Otherwise, a regression here could expose an unlisted rule without causing the current tests to fail.
### Issue 4
products/feature_flags/backend/api/test/test_feature_flag.py:8880
**Test signature lacks annotations**
The new `test_only_the_rules_in_the_setting_reject` method leaves its parameters and return value unannotated. This violates the repository directive to write Python function signatures as if mypy strict mode were enabled, so the annotations must be added before merging.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(feature-flags): enforce filters val..." | Re-trigger Greptile |
The structural tier decides separately from the cross-field one, and only the latter was covered. Uses two variant-key violations, which the serde guard type checks but does not reject, so they reach the structural tier.
gustavohstrassburger
approved these changes
Sep 4, 2026
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
Filters validation is finished but cannot be turned on. The switch is one boolean for all of it, so enabling the rules that are ready would also start rejecting the rules that live traffic still violates every day.
Two cleanup migrations emptied the backlog for some rules, and those rules now see zero violations. They are ready to reject. Others are not.
Part of #50084.
Changes
"*"behaves exactly like the old on.Rule ids are the ones already in the violation metrics and in the
codeof each error, so the setting reads the same as the dashboard.Note
No behavior changes on deploy. The variable is unset in production, which rejects nothing new. The checks that predate this work still reject, as they do today.
Mechanical: four existing tests point at the new setting name, and comments that named the old switch now name the rollout.
How did you test this code?
New tests in
TestFeatureFlagFiltersEnforcementcover the four states: a listed rule rejects, an unlisted rule is accepted, unset accepts, and"*"rejects. One more covers a request that violates both rules with one listed, and asserts the unlisted rule's message is absent from the response.They catch a regression nothing else did: dropping the per-rule filter at either decision point would make every violating write fail as soon as one rule goes live. Reverting that filter locally fails both accept cases.
Not checked: any behavior in a real environment. The rollout starts with the variable unset, so the first real signal is a rule going in and its violation counter falling.
Automatic notifications
Docs update
None.