🤖 fix: don't send default experiment value to backend

- useSendMessageOptions now passes undefined unless user explicitly overrides
- add useExperimentOverrideValue() helper
- harden localStorage parsing + add tests

Change-Id: I33d9f1c8d3bba4f083132c4f645c76328327726a
Signed-off-by: Thomas Kosiewski <[email protected]>

Author: ThomasK33

src/browser/contexts/ExperimentsContext.tsx

@@ -37,38 +37,42 @@ function subscribeToExperiment(experimentId: ExperimentId, callback: () => void)
 }
 
 /**
- * Get current experiment state from localStorage.
- * Returns the stored value or the default if not set.
+ * Get explicit localStorage override for an experiment.
+ * Returns undefined if no value is set or parsing fails.
  */
-function getExperimentSnapshot(experimentId: ExperimentId): boolean {
-  const experiment = EXPERIMENTS[experimentId];
+function getExperimentOverrideSnapshot(experimentId: ExperimentId): boolean | undefined {
   const key = getExperimentKey(experimentId);
 
   try {
     const stored = window.localStorage.getItem(key);
+    // Check for literal "undefined" string defensively - this can occur if
+    // JSON.stringify(undefined) is accidentally stored (it returns "undefined")
     if (stored === null || stored === "undefined") {
-      return experiment.enabledByDefault;
+      return undefined;
     }
-    return JSON.parse(stored) as boolean;
+
+    const parsed = JSON.parse(stored) as unknown;
+    return typeof parsed === "boolean" ? parsed : undefined;
   } catch {
-    return experiment.enabledByDefault;
+    return undefined;
   }
 }
 
+/**
+ * Get current experiment state from localStorage.
+ * Returns the stored value or the default if not set.
+ */
+function getExperimentSnapshot(experimentId: ExperimentId): boolean {
+  const experiment = EXPERIMENTS[experimentId];
+  return getExperimentOverrideSnapshot(experimentId) ?? experiment.enabledByDefault;
+}
+
 /**
  * Check if user has explicitly set a local override for an experiment.
  * Returns true if there's a value in localStorage (not using default).
  */
 function hasLocalOverride(experimentId: ExperimentId): boolean {
-  const key = getExperimentKey(experimentId);
-  try {
-    const stored = window.localStorage.getItem(key);
-    // Check for literal "undefined" string defensively - this can occur if
-    // JSON.stringify(undefined) is accidentally stored (it returns "undefined")
-    return stored !== null && stored !== "undefined";
-  } catch {
-    return false;
-  }
+  return getExperimentOverrideSnapshot(experimentId) !== undefined;
 }
 
 /**
@@ -210,6 +214,27 @@ export function useExperimentValue(experimentId: ExperimentId): boolean {
   return localEnabled;
 }
 
+/**
+ * Hook to read only an explicit local override for an experiment.
+ *
+ * Returns `undefined` when the user has not explicitly set a value in localStorage.
+ * This is important for user-overridable experiments: the backend can then apply
+ * the PostHog assignment instead of treating the default value as a user choice.
+ */
+export function useExperimentOverrideValue(experimentId: ExperimentId): boolean | undefined {
+  const subscribe = useCallback(
+    (callback: () => void) => subscribeToExperiment(experimentId, callback),
+    [experimentId]
+  );
+
+  const getSnapshot = useCallback(
+    () => getExperimentOverrideSnapshot(experimentId),
+    [experimentId]
+  );
+
+  return useSyncExternalStore(subscribe, getSnapshot, getSnapshot);
+}
+
 /**
  * Hook to get setter function for experiments.
  * Use this in components that need to toggle experiments (e.g., Settings).

src/browser/hooks/useExperiments.test.ts

@@ -0,0 +1,53 @@
+/**
+ * Tests for isExperimentEnabled()
+ *
+ * Key invariant:
+ * - For user-overridable experiments, absence of a localStorage entry must be treated as
+ *   "no explicit override" (undefined), so the backend can apply PostHog assignment.
+ */
+
+import { afterEach, beforeEach, describe, expect, test } from "bun:test";
+import { GlobalWindow } from "happy-dom";
+import { EXPERIMENT_IDS, getExperimentKey } from "@/common/constants/experiments";
+import { isExperimentEnabled } from "./useExperiments";
+
+describe("isExperimentEnabled", () => {
+  beforeEach(() => {
+    globalThis.window = new GlobalWindow() as unknown as Window & typeof globalThis;
+    globalThis.document = globalThis.window.document;
+    globalThis.window.localStorage.clear();
+  });
+
+  afterEach(() => {
+    globalThis.window = undefined as unknown as Window & typeof globalThis;
+    globalThis.document = undefined as unknown as Document;
+  });
+
+  test("returns undefined when no local override exists for a user-overridable experiment", () => {
+    expect(isExperimentEnabled(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT)).toBeUndefined();
+  });
+
+  test("returns boolean when local override exists", () => {
+    const key = getExperimentKey(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT);
+
+    globalThis.window.localStorage.setItem(key, JSON.stringify(true));
+    expect(isExperimentEnabled(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT)).toBe(true);
+
+    globalThis.window.localStorage.setItem(key, JSON.stringify(false));
+    expect(isExperimentEnabled(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT)).toBe(false);
+  });
+
+  test('treats literal "undefined" as no override', () => {
+    const key = getExperimentKey(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT);
+
+    globalThis.window.localStorage.setItem(key, "undefined");
+    expect(isExperimentEnabled(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT)).toBeUndefined();
+  });
+
+  test("treats non-boolean stored value as no override", () => {
+    const key = getExperimentKey(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT);
+
+    globalThis.window.localStorage.setItem(key, JSON.stringify("test"));
+    expect(isExperimentEnabled(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT)).toBeUndefined();
+  });
+});

src/browser/hooks/useExperiments.ts

@@ -5,6 +5,7 @@ import { type ExperimentId, EXPERIMENTS, getExperimentKey } from "@/common/const
 export {
   useExperiment,
   useExperimentValue,
+  useExperimentOverrideValue,
   useSetExperiment,
   useAllExperiments,
 } from "@/browser/contexts/ExperimentsContext";
@@ -14,7 +15,8 @@ export {
  * Use when you need a one-time read (e.g., constructing send options at send time)
  * or outside of React components.
  *
- * For reactive updates in React components, use useExperimentValue instead.
+ * For reactive updates in React components, use useExperimentValue (UI gating) or
+ * useExperimentOverrideValue (backend send options).
  *
  * IMPORTANT: For user-overridable experiments, returns `undefined` when no explicit
  * localStorage override exists. This signals to the backend to use the PostHog
@@ -30,18 +32,11 @@ export function isExperimentEnabled(experimentId: ExperimentId): boolean | undef
   // For user-overridable experiments: only return a value if user explicitly set one.
   // This allows the backend to use PostHog assignment when there's no override.
   if (experiment.userOverridable) {
-    const raw = window.localStorage.getItem(key);
-    // Check for null (never set) or literal "undefined" (defensive - see hasLocalOverride)
-    if (raw === null || raw === "undefined") {
-      return undefined; // Let backend use PostHog
-    }
-    try {
-      return JSON.parse(raw) as boolean;
-    } catch {
-      return undefined;
-    }
+    const stored = readPersistedState<unknown>(key, undefined);
+    return typeof stored === "boolean" ? stored : undefined;
   }
 
   // Non-overridable: always use default (these are local-only experiments)
-  return readPersistedState<boolean>(key, experiment.enabledByDefault);
+  const stored = readPersistedState<unknown>(key, experiment.enabledByDefault);
+  return typeof stored === "boolean" ? stored : experiment.enabledByDefault;
 }

src/browser/hooks/useSendMessageOptions.ts

@@ -13,7 +13,7 @@ import { getSendOptionsFromStorage } from "@/browser/utils/messages/sendOptions"
 import { enforceThinkingPolicy } from "@/browser/utils/thinking/policy";
 import { useProviderOptions } from "./useProviderOptions";
 import type { GatewayState } from "./useGatewayModels";
-import { useExperimentValue } from "./useExperiments";
+import { useExperimentOverrideValue } from "./useExperiments";
 import { EXPERIMENT_IDS } from "@/common/constants/experiments";
 
 /**
@@ -47,7 +47,7 @@ function constructSendMessageOptions(
   providerOptions: MuxProviderOptions,
   fallbackModel: string,
   gateway: GatewayState,
-  postCompactionContext: boolean
+  postCompactionContext: boolean | undefined
 ): SendMessageOptions {
   // Ensure model is always a valid string (defensive against corrupted localStorage)
   const rawModel =
@@ -110,8 +110,9 @@ export function useSendMessageOptions(workspaceId: string): SendMessageOptionsWi
   // Subscribe to gateway state so we re-render when user toggles gateway
   const gateway = useGateway();
 
-  // Subscribe to experiment state so toggles apply immediately
-  const postCompactionContext = useExperimentValue(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT);
+  // Subscribe to local override state so toggles apply immediately.
+  // If undefined, the backend will apply the PostHog assignment.
+  const postCompactionContext = useExperimentOverrideValue(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT);
 
   // Compute base model (canonical format) for UI components
   const rawModel =

src/node/services/workspaceService.ts

@@ -687,9 +687,8 @@ export class WorkspaceService extends EventEmitter {
     try {
       const metadata = await this.config.getAllWorkspaceMetadata();
 
-      // For list(), we use includePostCompaction from options since it's already resolved
-      // by the frontend based on experiment state. The frontend's useExperimentValue()
-      // handles userOverridable logic, so we trust the passed value.
+      // For list(), treat includePostCompaction as an explicit frontend override when provided.
+      // If it's undefined (e.g., user hasn't overridden), fall back to PostHog assignment.
       const postCompactionExperiment = EXPERIMENTS[EXPERIMENT_IDS.POST_COMPACTION_CONTEXT];
       let includePostCompaction: boolean;
       if (
@@ -1024,7 +1023,7 @@ export class WorkspaceService extends EventEmitter {
       }
 
       // Experiments: resolve flags respecting userOverridable setting.
-      // - If userOverridable && frontend provides a value → use frontend value (user's choice)
+      // - If userOverridable && frontend provides a value (explicit override) → use frontend value
       // - Else if remote evaluation enabled → use PostHog assignment
       // - Else → use frontend value (dev fallback) or default
       const postCompactionExperiment = EXPERIMENTS[EXPERIMENT_IDS.POST_COMPACTION_CONTEXT];