chore(storybook): support multivariate feature flag variants in stories#60775
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Size Change: 0 B Total Size: 80.8 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2108ec73ea
ℹ️ 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".
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
frontend/src/mocks/browser.tsx:63-71
**Variant state not cleared when navigating away from a multivariate story**
When a multivariate story dispatches `featureFlagLogic.actions.setFeatureFlags`, the kea reducer (with `{ persist: true }`) stores `{ 'flag': 'test_b' }` in memory (and potentially localStorage). When the next story uses the array form or a Record with only boolean entries, `setFeatureFlags` action is not dispatched, so the kea state is never replaced. Any component in that next story reading `featureFlagLogic.values.featureFlags` will still see the previous story's variant — a silent incorrect render in interactive Storybook.
Calling `featureFlagLogic.actions.setFeatureFlags` unconditionally (not only when `variantEntries.length > 0`) would make every Record-form invocation replace the kea state. The array form has the same gap, so you'd need the same treatment there to fully close the window.
### Issue 2 of 2
frontend/src/mocks/browser.tsx:65
**`is_debug: true` is written globally and never reset**
Once any multivariate story runs and mutates `window.POSTHOG_APP_CONTEXT.preflight.is_debug = true`, the flag stays set for the lifetime of the Storybook session (the spread creates a new object, but `appContext` still points into `POSTHOG_APP_CONTEXT`). Every subsequent story — including those without any feature-flag parameter — will now pass the `spyOnFeatureFlags` gate. In the current setup this is harmless because `advanced_disable_feature_flags: true` prevents posthog-js from ever calling `onFeatureFlags` with real data, but any code path that reads `getAppContext()?.preflight?.is_debug` for a non-flag purpose will start seeing `true` unexpectedly after the first multivariate story runs.
Reviews (1): Last reviewed commit: "chore(storybook): support multivariate f..." | Re-trigger Greptile |
2108ec7 to
cf724e6
Compare
cf724e6 to
939e806
Compare
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
2f859dd to
23f781f
Compare
23f781f to
4c1c008
Compare
Stories could only turn flags on as booleans via the `featureFlags`
parameter. Components gated on a multivariate flag variant (e.g. an
experiment arm) could not be rendered: imperatively calling
`featureFlagLogic.setFeatureFlags` is dropped in the visual-regression
runtime because `spyOnFeatureFlags` only merges variant values when the
context looks cloud/debug, and posthog-js's flag fetch clobbers anything
the decorator sets.
This makes the existing `featureFlags` story parameter accept a
`Record<string, string | boolean>` alongside the existing `string[]`:
- boolean entries still ride `persisted_feature_flags` (the always-merged
baseline)
- variant strings open the gate (mark the storybook context debug) and
inject through `featureFlagLogic`
- the test-only `posthog.init('fake_token')` branch sets
`advanced_disable_feature_flags: true` so posthog-js never fetches and
never clobbers the story-set flags
Adds the `setting-feature-flags-in-storybook` skill and a path rule so
story authors find this instead of re-discovering the trap.
Generated-By: PostHog Code
Task-Id: 6e671a32-ae34-485c-b398-82729020dec5
4c1c008 to
bf80cbd
Compare

Problem
Storybook stories can turn feature flags on via the
featureFlagsparameter, but that only expresses booleans. A component gated on a specific multivariate flag variant (e.g. an experiment arm) can't be rendered in a story — so its visual-regression snapshots come out empty, silently.The trap is subtle and bites every multivariate-flag story:
featureFlagLogic.actions.setFeatureFlags(...)imperatively from a story is dropped in the visual-regression runtime.spyOnFeatureFlagsonly merges variant values when the context lookscloud/is_debug/test, and the mocked_preflight(which hasis_debug: true) is never written back towindow.POSTHOG_APP_CONTEXT.preflight— so the gate stays closed.posthog-jsfetches/flagsand firesonFeatureFlags, clobbering whatever the story set back to empty.jest(NODE_ENV==='test'opens the gate), so unit tests stay green while the Storybook render is blank — easy to miss.Changes
Makes the existing
featureFlagsstory parameter accept aRecord<string, string | boolean>alongsidestring[]:persisted_feature_flags(the always-merged baselinefeatureFlagLogicreads on mount) — unchanged behaviour.spyOnFeatureFlagsgate (mark the storybook contextis_debug) and inject throughfeatureFlagLogic.posthog.init('fake_token')branch inloadPostHogJS.tsxnow setsadvanced_disable_feature_flags: true, so posthog-js never fetches flags and never clobbers story-set values. The real-token production init is untouched.Usage:
Also adds the
setting-feature-flags-in-storybookskill and a.claude/rulespath-rule so story authors find this instead of re-discovering the trap.How did you test this code?
I'm an agent (PostHog Code). I validated the mechanism live, not just in CI:
parameters: { featureFlags: { 'promoted-product': 'intent_plus' } }and no manual intervention, the entry rendered correctly; thecontrolarm correctly rendered nothing. Before this change the same story rendered an empty wrapper.advanced_disable_feature_flagschange is scoped to thefake_token(storybook/test) init branch only — the real-token init is unchanged.featureFlagsstories are unaffected (booleans never relied on the imperative path).Automatic notifications
Docs update
No public docs — this is internal Storybook/test tooling. The new in-repo skill documents it for agents and contributors.
🤖 Agent context
Extracted from #60677 (the promoted-product experiment, which stacks on top of this and is the first consumer) so the reusable Storybook harness change can be reviewed independently by frontend/infra owners. The root cause was diagnosed by reproducing in a real browser against running Storybook (jest masks it via
NODE_ENV==='test'), which pinned both thespyOnFeatureFlagsgate and the posthog-jsonFeatureFlagsclobber.Created with PostHog Code