Skip to content

Experiments: persist only explicit overrides so default flips reach existing installs #2157

Open

Description

@easonLiangWorldedtech

Problem

Every experiment default flip silently misses existing installs because the persisted experiments map records all keys, not just user overrides:

  • SettingsView puts the whole experiments map into the updateSettings payload on every Save (webview-ui/src/components/settings/SettingsView.tsx:455). cachedState.experiments is initialised from ExtensionState, which is the persisted map merged over the defaults.
  • webviewMessageHandler merges that map over experimentDefault and persists the result (src/core/webview/webviewMessageHandler.ts:843-851).
  • experiments.isEnabled treats a stored value as explicit: experimentsConfig[id] ?? experimentDefault[id] (src/shared/experiments.ts:42).

Consequence: a user who saved the settings form once has every experiment key stored at its then-current default. A later default flip in experimentConfigsMap only reaches installs whose persisted map has no value for that key. This is the rollout limitation documented in #1384: making chat-diff the default approval path only reaches installs that have never saved the settings form, and a stored preventFocusDisruption: false is indistinguishable from an explicit opt-out.

Why it matters

Any future experiment graduated by flipping its default gets the same behavior. It also makes rollouts hard to reason about: two users with identical explicit choices can end up with different experiment states depending on whether they ever pressed Save.

Proposed direction

Persist only explicit overrides instead of the merged map. Options:

  1. SettingsView sends only experiment ids whose value differs from experimentDefault (and the handler replaces the stored map instead of merging), or
  2. keep an explicit-overrides set (experimentsOverrides) and have isEnabled read that, falling back to the default when absent.

A migration that drops keys equal to the old default is not recommended: it cannot tell a default value from an explicit choice, and would silently re-enable experiments users intentionally disabled.

Acceptance criteria

  • A default flip in experimentConfigsMap reaches installs that have saved the settings form unless the user explicitly toggled that experiment.
  • Unit tests cover the SettingsView payload (only changed experiment keys), the handler persistence (stored overrides, not merged defaults) and isEnabled precedence.
  • No migration that guesses between default and explicit values.

Out of scope for #1384: that PR keeps the current persistence behavior and documents the rollout limitation in its description. Filed from the @edelauna review thread on #1384 (src/shared/experiments.ts:25).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions