diff --git a/growthbook/core.py b/growthbook/core.py index 9354932..6ee7bf2 100644 --- a/growthbook/core.py +++ b/growthbook/core.py @@ -705,8 +705,8 @@ def eval_prereqs(parentConditions: List[dict], evalContext: EvaluationContext) - return "cyclic" parent_condition = parentCondition.get("condition") - if parent_condition is None: - continue # Skip if no valid condition + if not isinstance(parent_condition, dict): + continue # Skip if missing or malformed (non-dict) condition if not evalCondition({'value': parentRes.value}, parent_condition, evalContext.global_ctx.saved_groups): if parentCondition.get("gate", False): diff --git a/growthbook/growthbook.py b/growthbook/growthbook.py index ff5f5e9..5603f2b 100644 --- a/growthbook/growthbook.py +++ b/growthbook/growthbook.py @@ -1308,10 +1308,15 @@ def _sync_user_ctx_from_instance(self) -> None: # refresh_sticky_buckets(); intentionally NOT mirrored here. `groups` # and `skip_all_experiments` have no setters today so they don't drift. - def _get_eval_context(self) -> EvaluationContext: - # Lazy refresh: ensure features are fresh before evaluation - self._ensure_fresh_features() - + def _build_eval_context(self) -> EvaluationContext: + """Assemble an EvaluationContext WITHOUT any side effects. + + Unlike `_get_eval_context`, this never triggers a feature refresh, so + it is safe to call from inside the feature-refresh / sticky-bucket flow + (see `_get_sticky_bucket_attributes`). Calling `_get_eval_context` there + would re-enter `_ensure_fresh_features` -> `load_features` for every + sticky-bucket identifier attribute, causing redundant feature reloads. + """ # Centralized sync (see _sync_user_ctx_from_instance for rationale). self._sync_user_ctx_from_instance() # global_ctx.options.url is not part of _user_ctx; still needs updating. @@ -1322,6 +1327,12 @@ def _get_eval_context(self) -> EvaluationContext: stack = StackContext(evaluated_features=set()) ) + def _get_eval_context(self) -> EvaluationContext: + # Lazy refresh: ensure features are fresh before evaluation, then build + # the (side-effect-free) context. + self._ensure_fresh_features() + return self._build_eval_context() + def eval_feature(self, key: str) -> FeatureResult: result = core_eval_feature(key=key, evalContext=self._get_eval_context(), @@ -1409,8 +1420,12 @@ def _get_sticky_bucket_attributes(self) -> dict: if not self.sticky_bucket_identifier_attributes: return attributes + # Build the context once, side-effect-free. Using _get_eval_context() + # here would re-trigger _ensure_fresh_features() -> load_features() on + # every attribute (this method itself runs inside the refresh flow). + eval_context = self._build_eval_context() for attr in self.sticky_bucket_identifier_attributes: - _, hash_value = _getHashValue(attr=attr, eval_context=self._get_eval_context()) + _, hash_value = _getHashValue(attr=attr, eval_context=eval_context) if hash_value: attributes[attr] = hash_value return attributes diff --git a/tests/test_growthbook.py b/tests/test_growthbook.py index b51f6a6..1e7b460 100644 --- a/tests/test_growthbook.py +++ b/tests/test_growthbook.py @@ -1162,6 +1162,58 @@ def mock_fetch_features(api_host, client_key, decryption_key=""): feature_repo.clear_cache() +def test_sticky_bucket_refresh_does_not_reload_features(mocker): + """refresh_sticky_buckets() must not trigger redundant feature reloads. + + Regression: _get_sticky_bucket_attributes() used to call _get_eval_context() + -- which runs _ensure_fresh_features() -> load_features() -- once per + sticky-bucket identifier attribute. On the set_attributes() path (where the + _is_updating_features guard is not set) that meant N redundant feature + reloads, each re-running set_features() + the sticky-bucket service lookup. + The fix routes sticky-bucket hashing through the side-effect-free + _build_eval_context(), so no reload happens during the refresh. + """ + feature_repo.clear_cache() + response = { + "features": { + "exp-feature": { + "defaultValue": 0, + "rules": [ + { + "key": "exp1", + "hashAttribute": "id", + "fallbackAttribute": "deviceId", + "variations": [0, 1], + "meta": [{"key": "0"}, {"key": "1"}], + } + ], + } + }, + "savedGroups": {}, + } + mocker.patch.object(feature_repo, '_fetch_features', return_value=response) + + service = InMemoryStickyBucketService() + gb = GrowthBook( + api_host="https://cdn.growthbook.io", + client_key="test-key", + sticky_bucket_service=service, + ) + try: + # Populate features (lazy load) so multiple sticky-bucket identifier + # attributes are derived from the rule above. + gb.get_feature_value("exp-feature", -1) + assert set(gb.sticky_bucket_identifier_attributes) == {"id", "deviceId"} + + # set_attributes() -> refresh_sticky_buckets() must not reload features. + spy = mocker.spy(gb, "load_features") + gb.set_attributes({"id": "user-123", "deviceId": "dev-abc"}) + assert spy.call_count == 0 + finally: + gb.destroy() + feature_repo.clear_cache() + + def test_multiple_instances_get_updated_on_cache_expiry(mocker): """Test that multiple GrowthBook instances all get updated when cache expires during evaluation""" mock_responses = [