Skip to content

Commit fbdf9c3

Browse files
committed
🤖 fix: non-React flows now respect PostHog experiment assignments
Previously, getSendOptionsFromStorage() (used by resume/creation flows) always passed the localStorage default to the backend, which treated any non-undefined value as an explicit user override for userOverridable experiments. Fix: isExperimentEnabled() now returns undefined for userOverridable experiments when user hasn't explicitly set a localStorage value. The backend already handles undefined correctly by falling through to PostHog assignment. Also addresses code review feedback: - Move telemetryService property declaration to top of ExperimentsService class - Add comment explaining the "undefined" string check in hasLocalOverride - Add docstring for getRemoteExperimentEnabled and move near other helpers - Document MUX_ENABLE_TELEMETRY_IN_DEV env var in Makefile Change-Id: I15e5360f7461cd62ad347fbb2e32b0e8dc4b873d Signed-off-by: Thomas Kosiewski <[email protected]>
1 parent 83677f1 commit fbdf9c3

File tree

4 files changed

+40
-9
lines changed

4 files changed

+40
-9
lines changed

Makefile

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@
2323
# AVOID CONDITIONAL BRANCHES (if/else) IN BUILD TARGETS AT ALL COSTS.
2424
# Branches reduce reproducibility - builds should fail fast with clear errors
2525
# if dependencies are missing, not silently fall back to different behavior.
26+
#
27+
# Telemetry in Development:
28+
# Telemetry is disabled by default in dev mode (MUX_DISABLE_TELEMETRY=1).
29+
# To enable it (e.g., for testing PostHog experiments), set:
30+
# MUX_ENABLE_TELEMETRY_IN_DEV=1 make dev
31+
# This single env var is sufficient - no need to also set MUX_DISABLE_TELEMETRY=0.
2632

2733
# Use PATH-resolved bash for portability across different systems.
2834
# - Windows: /usr/bin/bash doesn't exist in Chocolatey's make environment or GitHub Actions

src/browser/contexts/ExperimentsContext.tsx

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,24 +63,28 @@ function hasLocalOverride(experimentId: ExperimentId): boolean {
6363
const key = getExperimentKey(experimentId);
6464
try {
6565
const stored = window.localStorage.getItem(key);
66+
// Check for literal "undefined" string defensively - this can occur if
67+
// JSON.stringify(undefined) is accidentally stored (it returns "undefined")
6668
return stored !== null && stored !== "undefined";
6769
} catch {
6870
return false;
6971
}
7072
}
7173

7274
/**
73-
* Set experiment state to localStorage and dispatch sync event.
75+
* Convert PostHog experiment variant to boolean enabled state.
76+
* For experiments with control/test variants, "test" means enabled.
7477
*/
75-
7678
function getRemoteExperimentEnabled(value: string | boolean): boolean {
7779
if (typeof value === "boolean") {
7880
return value;
7981
}
80-
81-
// For now, our PostHog experiment uses control/test variants.
8282
return value === "test";
8383
}
84+
85+
/**
86+
* Set experiment state to localStorage and dispatch sync event.
87+
*/
8488
function setExperimentState(experimentId: ExperimentId, enabled: boolean): void {
8589
const key = getExperimentKey(experimentId);
8690

src/browser/hooks/useExperiments.ts

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,32 @@ export {
1616
*
1717
* For reactive updates in React components, use useExperimentValue instead.
1818
*
19+
* IMPORTANT: For user-overridable experiments, returns `undefined` when no explicit
20+
* localStorage override exists. This signals to the backend to use the PostHog
21+
* assignment instead of treating the default value as a user choice.
22+
*
1923
* @param experimentId - The experiment to check
20-
* @returns Whether the experiment is enabled
24+
* @returns Whether the experiment is enabled, or undefined if backend should decide
2125
*/
22-
export function isExperimentEnabled(experimentId: ExperimentId): boolean {
26+
export function isExperimentEnabled(experimentId: ExperimentId): boolean | undefined {
2327
const experiment = EXPERIMENTS[experimentId];
24-
return readPersistedState<boolean>(getExperimentKey(experimentId), experiment.enabledByDefault);
28+
const key = getExperimentKey(experimentId);
29+
30+
// For user-overridable experiments: only return a value if user explicitly set one.
31+
// This allows the backend to use PostHog assignment when there's no override.
32+
if (experiment.userOverridable) {
33+
const raw = window.localStorage.getItem(key);
34+
// Check for null (never set) or literal "undefined" (defensive - see hasLocalOverride)
35+
if (raw === null || raw === "undefined") {
36+
return undefined; // Let backend use PostHog
37+
}
38+
try {
39+
return JSON.parse(raw) as boolean;
40+
} catch {
41+
return undefined;
42+
}
43+
}
44+
45+
// Non-overridable: always use default (these are local-only experiments)
46+
return readPersistedState<boolean>(key, experiment.enabledByDefault);
2547
}

src/node/services/experimentsService.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ function isRecord(value: unknown): value is Record<string, unknown> {
4040
* - Avoid calling PostHog when telemetry is disabled
4141
*/
4242
export class ExperimentsService {
43+
private readonly telemetryService: TelemetryService;
4344
private readonly muxHome: string;
4445
private readonly cacheFilePath: string;
4546
private readonly cacheTtlMs: number;
@@ -60,8 +61,6 @@ export class ExperimentsService {
6061
this.cacheTtlMs = options.cacheTtlMs ?? DEFAULT_CACHE_TTL_MS;
6162
}
6263

63-
private readonly telemetryService: TelemetryService;
64-
6564
async initialize(): Promise<void> {
6665
if (this.cacheLoaded) {
6766
return;

0 commit comments

Comments
 (0)