fix(feature-flags): preserve multivariate shape when syncing from API#60146
Conversation
`sync-feature-flags` used a fixed boolean filter when creating local flags from the `/flags?v=2` response, so multivariate flags in production were recreated locally as plain boolean flags. Detect the evaluated `variant` in the response and seed the new flag as multivariate with that variant at 100% rollout — the API only returns the variant the supplied distinct_id resolved to, so the rest must still be added by hand, but the flag stays multivariate instead of being silently downgraded. Generated-By: PostHog Code Task-Id: fa51d026-bc22-4025-bf43-1918e26cd4ec
|
🎭 Playwright didn't run on this PR — your changes touch code that could affect E2E behavior, but Playwright is opt-in via label now to keep CI cost down. Add the Most PRs don't need this. Real regressions still get caught on master and fix-forward. |
|
Reviews (1): Last reviewed commit: "fix(feature-flags): preserve multivariat..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83733664e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # the user fill in the rest — better than silently dropping to boolean. | ||
| variant = flag_data.get("variant") | ||
| base: dict = {"groups": [{"properties": [], "rollout_percentage": 100}], "payloads": {}} | ||
| if variant: |
There was a problem hiding this comment.
Handle empty-string variants as multivariate
Use an explicit null check here instead of a truthiness check. The /flags?v=2 response and other call sites in the codebase treat variant is not None as the signal for multivariate results, so a valid-but-empty variant key ("") will currently skip multivariate and fall back to a boolean-shaped flag, recreating the downgrade this change is meant to prevent for that input class. Switching to if variant is not None: preserves multivariate shape consistently.
Useful? React with 👍 / 👎.
Problem
Running
sync-feature-flags(i.e.python manage.py sync_feature_flags_from_api) against the production/flags?v=2endpoint always created new local feature flags as boolean, even when the production flag was multivariate. That made it impossible to bootstrap a local dev project with multivariate flags — they'd all come back as plain booleans and the existing multivariate workflow would break.Changes
When the
/flags?v=2response includes a non-nullvariantfor an enabled flag, seed the newly-created local flag as multivariate with that variant at 100% rollout instead of as a boolean flag. The API only returns the single variant the supplieddistinct_idresolved to, so the flag still needs the remaining variants filled in by hand — but it no longer gets silently downgraded to boolean.Boolean flags (variant is null/absent) are unchanged.
How did you test this code?
I'm an agent. I made the targeted code change to
_build_filtersinposthog/management/commands/sync_feature_flags_from_api.pyand validated the file parses. I did not run the management command end-to-end against the live/flags?v=2endpoint, and did not add an automated test for this command (there isn't one today). Reviewer should sanity-check by running the command locally with adistinct_idthat resolves multivariate flags and confirming the new local flags have amultivariate.variantsblock.Publish to changelog?
no
Docs update
No docs change needed — this is a dev-tooling fix, not a product feature.
🤖 Agent context
sync-feature-flagscreates boolean flags and messes up multivariate ones." Reading the command, the only place that writesfiltersis the new-flag branch, which hard-coded a boolean shape. Existing flags only get theiractivefield touched, so we did not change that path./api/feature_flag/local_evaluation, but that needs a personal API key which the command does not have. Settled on extracting the single observed variant from the public/flags?v=2response, since that is enough to keep the flag multivariate.controlvariant alongside the observed one. Rejected — the observed variant could be anything (blue,red, etc.) and addingcontrolat 0% would be misleading.Created with PostHog Code