🤖 feat: Add userOverridable and showInSettings for experiments

ThomasK33 · ThomasK33 · commit fd97c9a436e5 · 2025-12-16T18:59:49.000+01:00
Allow per-experiment control over whether users can override remote
PostHog assignments and whether experiments appear in Settings.

ExperimentDefinition changes:
- userOverridable?: boolean - when true, local toggle takes precedence
- showInSettings?: boolean - when false, hide from Settings UI

Resolution priority (when userOverridable=true):
1. Local localStorage toggle (user's explicit choice)
2. Remote PostHog assignment
3. Default (enabledByDefault)

Implementation:
- experiments.ts: Add new optional fields, set POST_COMPACTION_CONTEXT
  as userOverridable=true
- ExperimentsContext: hasLocalOverride() helper, updated useExperimentValue()
  and useAllExperiments() to respect userOverridable
- ExperimentsSection: Filter by showInSettings, enable toggle when canOverride
- WorkspaceService: Respect userOverridable in both list() and sendMessage()

Change-Id: I3afc8514c74151b8b72991aa13ab98296cfd19bb
Signed-off-by: Thomas Kosiewski &lt;tk@coder.com&gt;
diff --git a/src/browser/components/Settings/sections/ExperimentsSection.tsx b/src/browser/components/Settings/sections/ExperimentsSection.tsx
@@ -1,6 +1,7 @@
-import React, { useCallback } from "react";
+import React, { useCallback, useMemo } from "react";
 import { useExperiment, useRemoteExperimentValue } from "@/browser/contexts/ExperimentsContext";
 import {
+  EXPERIMENTS,
   getExperimentList,
   EXPERIMENT_IDS,
   type ExperimentId,
@@ -16,21 +17,24 @@ interface ExperimentRowProps {
 }
 
 function ExperimentRow(props: ExperimentRowProps) {
+  const experiment = EXPERIMENTS[props.experimentId];
   const [enabled, setEnabled] = useExperiment(props.experimentId);
   const remote = useRemoteExperimentValue(props.experimentId);
   const isRemoteControlled = remote ? remote.source !== "disabled" : false;
+  const canOverride = experiment.userOverridable === true;
   const { onToggle } = props;
 
   const handleToggle = useCallback(
     (value: boolean) => {
-      if (isRemoteControlled) {
+      // Allow toggle if not remote-controlled OR if user can override
+      if (isRemoteControlled && !canOverride) {
         return;
       }
 
       setEnabled(value);
       onToggle?.(value);
     },
-    [isRemoteControlled, setEnabled, onToggle]
+    [isRemoteControlled, canOverride, setEnabled, onToggle]
   );
 
   return (
@@ -40,13 +44,14 @@ function ExperimentRow(props: ExperimentRowProps) {
         <div className="text-muted mt-0.5 text-xs">{props.description}</div>
         {isRemoteControlled ? (
           <div className="text-muted mt-0.5 text-xs">
-            PostHog: {remote?.value ?? "loading"} ({remote?.source})
+            PostHog: {String(remote?.value ?? "loading")} ({remote?.source})
+            {canOverride ? " • overridable" : null}
           </div>
         ) : null}
       </div>
       <Switch
         checked={enabled}
-        disabled={isRemoteControlled}
+        disabled={isRemoteControlled && !canOverride}
         onCheckedChange={handleToggle}
         aria-label={`Toggle ${props.name}`}
       />
@@ -55,9 +60,15 @@ function ExperimentRow(props: ExperimentRowProps) {
 }
 
 export function ExperimentsSection() {
-  const experiments = getExperimentList();
+  const allExperiments = getExperimentList();
   const { refreshWorkspaceMetadata } = useWorkspaceContext();
 
+  // Filter to only show experiments where showInSettings !== false
+  const experiments = useMemo(
+    () => allExperiments.filter((exp) => exp.showInSettings !== false),
+    [allExperiments]
+  );
+
   // When post-compaction experiment is toggled, refresh metadata to fetch/clear bundled state
   const handlePostCompactionToggle = useCallback(() => {
     void refreshWorkspaceMetadata();
diff --git a/src/browser/contexts/ExperimentsContext.tsx b/src/browser/contexts/ExperimentsContext.tsx
@@ -38,6 +38,7 @@ function subscribeToExperiment(experimentId: ExperimentId, callback: () => void)
 
 /**
  * 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];
@@ -54,6 +55,20 @@ function getExperimentSnapshot(experimentId: ExperimentId): boolean {
   }
 }
 
+/**
+ * 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);
+    return stored !== null && stored !== "undefined";
+  } catch {
+    return false;
+  }
+}
+
 /**
  * Set experiment state to localStorage and dispatch sync event.
  */
@@ -155,10 +170,16 @@ export function ExperimentsProvider(props: { children: React.ReactNode }) {
  * Uses useSyncExternalStore for efficient, selective re-renders.
  * Only re-renders when THIS specific experiment changes.
  *
+ * Resolution priority:
+ * - If userOverridable && user has explicitly set a local value → use local
+ * - If remote PostHog assignment exists → use remote
+ * - Otherwise → use local (which may be default)
+ *
  * @param experimentId - The experiment to subscribe to
  * @returns Whether the experiment is enabled
  */
 export function useExperimentValue(experimentId: ExperimentId): boolean {
+  const experiment = EXPERIMENTS[experimentId];
   const subscribe = useCallback(
     (callback: () => void) => subscribeToExperiment(experimentId, callback),
     [experimentId]
@@ -171,10 +192,17 @@ export function useExperimentValue(experimentId: ExperimentId): boolean {
   const context = useContext(ExperimentsContext);
   const remote = context?.remoteExperiments?.[experimentId];
 
+  // User-overridable: local wins if explicitly set
+  if (experiment.userOverridable && hasLocalOverride(experimentId)) {
+    return localEnabled;
+  }
+
+  // Remote assignment (if available and not disabled)
   if (remote && remote.source !== "disabled" && remote.value !== null) {
     return getRemoteExperimentEnabled(remote.value);
   }
 
+  // Fallback to local (which may be default)
   return localEnabled;
 }
 
@@ -237,20 +265,25 @@ export function useAllExperiments(): Record<ExperimentId, boolean> {
 
   const getSnapshot = useCallback(() => {
     const result: Partial<Record<ExperimentId, boolean>> = {};
+
     for (const exp of experiments) {
-      result[exp.id] = getExperimentSnapshot(exp.id);
-    }
+      const localValue = getExperimentSnapshot(exp.id);
+      const remote = remoteExperiments?.[exp.id];
 
-    if (remoteExperiments) {
-      for (const [experimentId, remote] of Object.entries(remoteExperiments) as Array<
-        [ExperimentId, ExperimentValue]
-      >) {
-        if (!remote || remote.source === "disabled" || remote.value === null) {
-          continue;
-        }
+      // User-overridable: local wins if explicitly set
+      if (exp.userOverridable && hasLocalOverride(exp.id)) {
+        result[exp.id] = localValue;
+        continue;
+      }
 
-        result[experimentId] = getRemoteExperimentEnabled(remote.value);
+      // Remote assignment (if available and not disabled)
+      if (remote && remote.source !== "disabled" && remote.value !== null) {
+        result[exp.id] = getRemoteExperimentEnabled(remote.value);
+        continue;
       }
+
+      // Fallback to local (which may be default)
+      result[exp.id] = localValue;
     }
 
     return result as Record<ExperimentId, boolean>;
diff --git a/src/common/constants/experiments.ts b/src/common/constants/experiments.ts
@@ -17,6 +17,16 @@ export interface ExperimentDefinition {
   description: string;
   /** Default state - false means disabled by default */
   enabledByDefault: boolean;
+  /**
+   * When true, user can override remote PostHog assignment via Settings toggle.
+   * When false (default), remote assignment is authoritative.
+   */
+  userOverridable?: boolean;
+  /**
+   * When false, experiment is hidden from Settings → Experiments.
+   * Defaults to true. Use false for invisible A/B tests.
+   */
+  showInSettings?: boolean;
 }
 
 /**
@@ -29,6 +39,8 @@ export const EXPERIMENTS: Record<ExperimentId, ExperimentDefinition> = {
     name: "Post-Compaction Context",
     description: "Re-inject plan file and edited file diffs after compaction to preserve context",
     enabledByDefault: false,
+    userOverridable: true, // User can opt-out via Settings
+    showInSettings: true,
   },
 };
 
diff --git a/src/node/services/telemetryService.featureFlags.test.ts b/src/node/services/telemetryService.featureFlags.test.ts
@@ -17,41 +17,77 @@ describe("TelemetryService feature flag properties", () => {
   });
 
   test("capture includes $feature/<flagKey> properties when set", () => {
-    const telemetry = new TelemetryService(tempDir);
-
-    const capture = mock((_args: unknown) => undefined);
-
-    // NOTE: TelemetryService only checks that client + distinctId are set.
-    // We set them directly to avoid any real network calls.
-    // @ts-expect-error - Accessing private property for test
-    telemetry.client = { capture };
-    // @ts-expect-error - Accessing private property for test
-    telemetry.distinctId = "distinct-id";
-
-    telemetry.setFeatureFlagVariant("post-compaction-context", "test");
-
-    const payload: TelemetryEventPayload = {
-      event: "message_sent",
-      properties: {
-        workspaceId: "workspace-id",
-        model: "test-model",
-        mode: "exec",
-        message_length_b2: 128,
-        runtimeType: "local",
-        frontendPlatform: {
-          userAgent: "ua",
-          platform: "platform",
-        },
-        thinkingLevel: "off",
-      },
+    // The capture method checks isTelemetryDisabledByEnv which checks NODE_ENV=test,
+    // JEST_WORKER_ID, VITEST, CI, etc. We need to temporarily clear these for the test.
+    const savedEnv = {
+      NODE_ENV: process.env.NODE_ENV,
+      JEST_WORKER_ID: process.env.JEST_WORKER_ID,
+      VITEST: process.env.VITEST,
+      CI: process.env.CI,
+      GITHUB_ACTIONS: process.env.GITHUB_ACTIONS,
+      MUX_DISABLE_TELEMETRY: process.env.MUX_DISABLE_TELEMETRY,
+      MUX_E2E: process.env.MUX_E2E,
+      TEST_INTEGRATION: process.env.TEST_INTEGRATION,
     };
 
-    telemetry.capture(payload);
+    // Clear all telemetry-disabling env vars
+    process.env.NODE_ENV = "production";
+    delete process.env.JEST_WORKER_ID;
+    delete process.env.VITEST;
+    delete process.env.CI;
+    delete process.env.GITHUB_ACTIONS;
+    delete process.env.MUX_DISABLE_TELEMETRY;
+    delete process.env.MUX_E2E;
+    delete process.env.TEST_INTEGRATION;
+
+    try {
+      const telemetry = new TelemetryService(tempDir);
+
+      const capture = mock((_args: unknown) => undefined);
+
+      // NOTE: TelemetryService only checks that client + distinctId are set.
+      // We set them directly to avoid any real network calls.
+      // @ts-expect-error - Accessing private property for test
+      telemetry.client = { capture };
+      // @ts-expect-error - Accessing private property for test
+      telemetry.distinctId = "distinct-id";
+
+      telemetry.setFeatureFlagVariant("post-compaction-context", "test");
+
+      const payload: TelemetryEventPayload = {
+        event: "message_sent",
+        properties: {
+          workspaceId: "workspace-id",
+          model: "test-model",
+          mode: "exec",
+          message_length_b2: 128,
+          runtimeType: "local",
+          frontendPlatform: {
+            userAgent: "ua",
+            platform: "platform",
+          },
+          thinkingLevel: "off",
+        },
+      };
+
+      telemetry.capture(payload);
 
-    expect(capture).toHaveBeenCalled();
+      expect(capture).toHaveBeenCalled();
 
-    const call = capture.mock.calls[0]?.[0] as { properties?: Record<string, unknown> } | undefined;
-    expect(call?.properties).toBeDefined();
-    expect(call?.properties?.["$feature/post-compaction-context"]).toBe("test");
+      const call = capture.mock.calls[0]?.[0] as
+        | { properties?: Record<string, unknown> }
+        | undefined;
+      expect(call?.properties).toBeDefined();
+      expect(call?.properties?.["$feature/post-compaction-context"]).toBe("test");
+    } finally {
+      // Restore all env vars
+      for (const [key, value] of Object.entries(savedEnv)) {
+        if (value === undefined) {
+          delete process.env[key];
+        } else {
+          process.env[key] = value;
+        }
+      }
+    }
   });
 });
diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts
@@ -14,7 +14,7 @@ import type { AIService } from "@/node/services/aiService";
 import type { InitStateManager } from "@/node/services/initStateManager";
 import type { ExtensionMetadataService } from "@/node/services/ExtensionMetadataService";
 import type { ExperimentsService } from "@/node/services/experimentsService";
-import { EXPERIMENT_IDS } from "@/common/constants/experiments";
+import { EXPERIMENT_IDS, EXPERIMENTS } from "@/common/constants/experiments";
 import type { MCPServerManager } from "@/node/services/mcpServerManager";
 import { createRuntime, IncompatibleRuntimeError } from "@/node/runtime/runtimeFactory";
 import { validateWorkspaceName } from "@/common/utils/validation/workspaceValidation";
@@ -687,10 +687,26 @@ export class WorkspaceService extends EventEmitter {
     try {
       const metadata = await this.config.getAllWorkspaceMetadata();
 
-      const includePostCompaction =
-        this.experimentsService?.isRemoteEvaluationEnabled() === true
-          ? this.experimentsService.isExperimentEnabled(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT)
-          : options?.includePostCompaction === true;
+      // 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.
+      const postCompactionExperiment = EXPERIMENTS[EXPERIMENT_IDS.POST_COMPACTION_CONTEXT];
+      let includePostCompaction: boolean;
+      if (
+        postCompactionExperiment.userOverridable &&
+        options?.includePostCompaction !== undefined
+      ) {
+        // User-overridable: trust frontend value
+        includePostCompaction = options.includePostCompaction;
+      } else if (this.experimentsService?.isRemoteEvaluationEnabled() === true) {
+        // Remote evaluation: use PostHog assignment
+        includePostCompaction = this.experimentsService.isExperimentEnabled(
+          EXPERIMENT_IDS.POST_COMPACTION_CONTEXT
+        );
+      } else {
+        // Fallback to frontend value or false
+        includePostCompaction = options?.includePostCompaction === true;
+      }
 
       if (!includePostCompaction) {
         return metadata;
@@ -1007,12 +1023,26 @@ export class WorkspaceService extends EventEmitter {
         void this.updateRecencyTimestamp(workspaceId, messageTimestamp);
       }
 
-      // Experiments: resolve backend-authoritative flags when telemetry is enabled.
-      // When telemetry is disabled, fall back to the renderer-provided experiment toggles.
-      const postCompactionContextEnabled =
-        this.experimentsService?.isRemoteEvaluationEnabled() === true
-          ? this.experimentsService.isExperimentEnabled(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT)
-          : options?.experiments?.postCompactionContext;
+      // Experiments: resolve flags respecting userOverridable setting.
+      // - If userOverridable && frontend provides a value → use frontend value (user's choice)
+      // - Else if remote evaluation enabled → use PostHog assignment
+      // - Else → use frontend value (dev fallback) or default
+      const postCompactionExperiment = EXPERIMENTS[EXPERIMENT_IDS.POST_COMPACTION_CONTEXT];
+      const frontendValue = options?.experiments?.postCompactionContext;
+
+      let postCompactionContextEnabled: boolean | undefined;
+      if (postCompactionExperiment.userOverridable && frontendValue !== undefined) {
+        // User-overridable: trust frontend value (user's explicit choice)
+        postCompactionContextEnabled = frontendValue;
+      } else if (this.experimentsService?.isRemoteEvaluationEnabled() === true) {
+        // Remote evaluation: use PostHog assignment
+        postCompactionContextEnabled = this.experimentsService.isExperimentEnabled(
+          EXPERIMENT_IDS.POST_COMPACTION_CONTEXT
+        );
+      } else {
+        // Fallback to frontend value (dev mode or telemetry disabled)
+        postCompactionContextEnabled = frontendValue;
+      }
 
       const resolvedOptions =
         postCompactionContextEnabled === undefined
