Merge pull request #8759 from Couchers-org/backend/refactor/openfeature-style-flag-methods

Backend: move feature-flag checks onto context with OpenFeature-style API

Author: aapeliv

app/backend/src/couchers/context.py

@@ -2,6 +2,8 @@
 
 import grpc
 
+from couchers import experimentation
+from couchers.config import config
 from couchers.i18n import LocalizationContext
 
 if TYPE_CHECKING:
@@ -179,6 +181,36 @@ def token(self) -> str:
     def localization(self) -> LocalizationContext:
         return self.__localization
 
+    # Feature-flag evaluation methods mirror the OpenFeature evaluation API. The in-code default is
+    # honored even for flags not yet set up in GrowthBook, since get_feature_value falls back to it.
+    def get_boolean_value(self, flag_key: str, default: bool) -> bool:
+        if config["EXPERIMENTATION_PASS_ALL_GATES"]:
+            return True
+        return self._get_feature_value(flag_key, default)
+
+    def get_string_value(self, flag_key: str, default: str) -> str:
+        return self._get_feature_value(flag_key, default)
+
+    def get_integer_value(self, flag_key: str, default: int) -> int:
+        return self._get_feature_value(flag_key, default)
+
+    def get_float_value(self, flag_key: str, default: float) -> float:
+        return self._get_feature_value(flag_key, default)
+
+    def get_object_value[T](self, flag_key: str, default: T) -> T:
+        return self._get_feature_value(flag_key, default)
+
+    def _get_feature_value[T](self, flag_key: str, default: T) -> T:
+        if not config["EXPERIMENTATION_ENABLED"]:
+            return default
+        return self._get_growthbook().get_feature_value(flag_key, default)  # type: ignore[no-any-return]
+
+    def _get_growthbook(self) -> GrowthBook:
+        if self._growthbook is None:
+            # _user_id is None when logged out: evaluate anonymously, falling through to defaults.
+            self._growthbook = experimentation._create_evaluator(self._user_id)
+        return self._growthbook
+
 
 def make_interactive_context(
     grpc_context: grpc.ServicerContext,

app/backend/src/couchers/db.py

@@ -6,6 +6,7 @@
 from contextlib import contextmanager
 from os import getpid
 from threading import get_ident
+from typing import TYPE_CHECKING
 
 from alembic import command
 from alembic.config import Config
@@ -19,7 +20,6 @@
 
 from couchers.config import config
 from couchers.constants import SERVER_THREADS, WORKER_THREADS
-from couchers.context import CouchersContext
 from couchers.models import (
     Cluster,
     ClusterRole,
@@ -33,6 +33,9 @@
 )
 from couchers.sql import where_users_column_visible
 
+if TYPE_CHECKING:
+    from couchers.context import CouchersContext
+
 # Register psycopg (psycopg3) as the default driver for postgresql:// URLs
 # This must happen before any engine is created
 registry.register("postgresql", "sqlalchemy.dialects.postgresql.psycopg", "PGDialect_psycopg")

app/backend/src/couchers/experimentation.py

@@ -2,12 +2,17 @@
 Experimentation framework for feature flags and experiments.
 
 Uses GrowthBook under the hood, but abstracts the implementation details.
+
+Don't evaluate flags by calling into this module directly - go through the CouchersContext methods
+(context.get_boolean_value, get_string_value, etc.), which own the per-request evaluator cache and
+the enabled / pass-all-gates gating. The underscore-prefixed helpers here are internal to that
+wiring; setup_experimentation() is the only public entry point, called once at process startup.
 """
 
 import json
 import logging
 import threading
-from typing import TYPE_CHECKING, Any
+from typing import Any
 
 import urllib3
 from growthbook import GrowthBook
@@ -18,9 +23,6 @@
 from couchers.db import session_scope
 from couchers.models.logging import ExperimentExposure
 
-if TYPE_CHECKING:
-    from couchers.context import CouchersContext
-
 logger = logging.getLogger(__name__)
 
 _REFRESH_INTERVAL_SECONDS = 60
@@ -110,13 +112,6 @@ def setup_experimentation() -> None:
     logger.info(f"Experimentation integration test: gate 'test_growthbook_integration' = {test_gate_result}")
 
 
-def _check_initialized() -> None:
-    if config["EXPERIMENTATION_ENABLED"] and not _initialized:
-        raise ExperimentationNotInitializedError(
-            "Experimentation is not initialized - call setup_experimentation() first"
-        )
-
-
 def _record_exposure(user_id: int, experiment: Experiment, result: Result, **_: Any) -> None:
     data = {
         "experiment_name": experiment.name,
@@ -144,59 +139,34 @@ def _record_exposure(user_id: int, experiment: Experiment, result: Result, **_:
         session.execute(stmt)
 
 
-def _get_growthbook(context: CouchersContext) -> GrowthBook:
+def _create_evaluator(user_id: int | None) -> GrowthBook:
     """
-    Get or create a cached GrowthBook instance for the given context.
-
-    Reads the in-memory feature snapshot maintained by the background refresh
-    thread - never does HTTP from the request path. Constructing without
-    `client_key` keeps the GrowthBook a pure evaluator: no callback
-    registration on the library's process-wide singleton.
-    """
-    gb = context._growthbook
-    if gb is None:
-        with _state_lock:
-            features = _state["features"]
-            saved_groups = _state["savedGroups"]
-
-        user_id = context.user_id
-
-        def on_experiment_viewed(experiment: Experiment, result: Result, **kwargs: Any) -> None:
-            _record_exposure(user_id, experiment, result)
-
-        gb = GrowthBook(
-            attributes={"id": str(user_id)},
-            features=features,
-            savedGroups=saved_groups,
-            on_experiment_viewed=on_experiment_viewed,
-        )
-        context._growthbook = gb
-    return gb
+    Build a per-request GrowthBook evaluator over the current feature snapshot.
 
+    Pass user_id=None for an anonymous (logged-out) evaluation: with no `id` attribute GrowthBook
+    can't bucket the user, so experiments and percentage rollouts are skipped and flags fall
+    through to their defaults. No exposure is recorded without a user.
 
-def check_gate(context: CouchersContext, gate_name: str) -> bool:
+    Reads the in-memory snapshot maintained by the background refresh thread - never does HTTP
+    from the request path. Constructing without `client_key` keeps the GrowthBook a pure
+    evaluator: no callback registration on the library's process-wide singleton. The caller is
+    responsible for caching this for the lifetime of a request.
     """
-    Check if a feature gate is enabled for the user in this context.
-
-    Returns False if experimentation is disabled, True if EXPERIMENTATION_PASS_ALL_GATES is set.
-    """
-    _check_initialized()
-    if config["EXPERIMENTATION_PASS_ALL_GATES"]:
-        return True
-    if not config["EXPERIMENTATION_ENABLED"]:
-        return False
-    return _get_growthbook(context).is_on(gate_name)
-
+    if not _initialized:
+        raise ExperimentationNotInitializedError(
+            "Experimentation is not initialized - call setup_experimentation() first"
+        )
+    with _state_lock:
+        features = _state["features"]
+        saved_groups = _state["savedGroups"]
 
-def get_feature_value[T](context: CouchersContext, feature_name: str, default: T) -> T:
-    """
-    Get the value of a feature for the user in this context.
+    def on_experiment_viewed(experiment: Experiment, result: Result, **kwargs: Any) -> None:
+        if user_id is not None:
+            _record_exposure(user_id, experiment, result)
 
-    Use this for non-boolean features: strings, numbers, dicts, experiment variations,
-    dynamic configs - anything other than a simple on/off gate. The default's type
-    determines the return type and is returned verbatim when experimentation is disabled.
-    """
-    _check_initialized()
-    if not config["EXPERIMENTATION_ENABLED"]:
-        return default
-    return _get_growthbook(context).get_feature_value(feature_name, default)  # type: ignore[no-any-return]
+    return GrowthBook(
+        attributes={"id": str(user_id)} if user_id is not None else {},
+        features=features,
+        savedGroups=saved_groups,
+        on_experiment_viewed=on_experiment_viewed,
+    )

app/backend/src/couchers/servicers/account.py

@@ -27,7 +27,6 @@
     verify_token,
 )
 from couchers.event_log import log_event
-from couchers.experimentation import check_gate
 from couchers.helpers.completed_profile import has_completed_profile
 from couchers.helpers.geoip import geoip_approximate_location
 from couchers.helpers.strong_verification import get_strong_verification_fields
@@ -169,7 +168,7 @@ def GetAccountInfo(
 
         # Test experimentation integration - check if user is in the test gate
         # Create 'test_growthbook_integration' in GrowthBook to test
-        test_gate = check_gate(context, "test_growthbook_integration")
+        test_gate = context.get_boolean_value("test_growthbook_integration", default=False)
         logger.info(f"Experimentation gate 'test_growthbook_integration' for user {user.id}: {test_gate}")
 
         should_show_donation_banner = DONATION_DRIVE_START is not None and (

app/backend/src/couchers/sql.py

@@ -4,7 +4,6 @@
 from sqlalchemy.orm import InstrumentedAttribute, aliased
 from sqlalchemy.sql import Select, exists, union
 
-from couchers.context import CouchersContext
 from couchers.models import (
     ModerationState,
     ModerationVisibility,
@@ -16,6 +15,7 @@
 from couchers.utils import is_valid_email, is_valid_user_id, is_valid_username
 
 if TYPE_CHECKING:
+    from couchers.context import CouchersContext
     from couchers.materialized_views import LiteUser
     from couchers.models.moderation import ModeratedContent
 

app/backend/src/tests/test_experimentation.py

@@ -0,0 +1,43 @@
+import pytest
+
+from couchers import experimentation
+from couchers.config import config
+from couchers.context import make_background_user_context, make_logged_out_context
+from couchers.i18n import LocalizationContext
+
+
+@pytest.fixture
+def experimentation_snapshot(monkeypatch):
+    """Enable experimentation with an in-memory feature snapshot for evaluation."""
+    monkeypatch.setattr(experimentation, "_initialized", True)
+    features = {
+        # A rollout with explicit coverage: bucketing needs a hash, so anonymous (logged-out) users
+        # are excluded even at 100% coverage and get the feature's default value instead.
+        "rollout_flag": {"defaultValue": "control", "rules": [{"force": "treatment", "coverage": 1.0}]},
+        # A global force with no coverage: applies to everyone, including anonymous users.
+        "global_flag": {"defaultValue": False, "rules": [{"force": True}]},
+    }
+    monkeypatch.setattr(experimentation, "_state", {"features": features, "savedGroups": {}})
+    monkeypatch.setitem(config, "EXPERIMENTATION_ENABLED", True)
+    monkeypatch.setitem(config, "EXPERIMENTATION_PASS_ALL_GATES", False)
+
+
+def test_logged_in_user_is_bucketed_into_rollout(experimentation_snapshot):
+    context = make_background_user_context(123)
+    assert context.get_string_value("rollout_flag", "fallback") == "treatment"
+
+
+def test_anonymous_user_excluded_from_rollout_gets_feature_default(experimentation_snapshot):
+    context = make_logged_out_context(LocalizationContext.en_utc())
+    # Previously this raised NotLoggedInContextException via context.user_id.
+    assert context.get_string_value("rollout_flag", "fallback") == "control"
+
+
+def test_anonymous_user_still_gets_global_force_on_flag(experimentation_snapshot):
+    context = make_logged_out_context(LocalizationContext.en_utc())
+    assert context.get_boolean_value("global_flag", default=False) is True
+
+
+def test_unknown_feature_returns_in_code_default(experimentation_snapshot):
+    context = make_logged_out_context(LocalizationContext.en_utc())
+    assert context.get_string_value("does_not_exist", "my_default") == "my_default"