fix: sync saved groups to the evaluation context on payload refresh

[vazarkevych]

Saved groups from a refreshed feature payload could silently never reach
$inGroup / $notInGroup evaluation.

The bug

Assigning self._saved_groups only rebinds the instance attribute — the
evaluation context keeps referencing the previous dict. The global context was
therefore only refreshed as a side effect of set_features(), which assigns
_global_ctx.saved_groups on its way through.

That side effect does not always happen:

• Streaming (SSE) and stale-while-revalidate. _ensure_fresh_features()
  returns early in both modes, so evaluation does no inline refresh. A
  payload's saved groups never took effect until some later call happened to
  invoke set_features().
• A payload carrying savedGroups but no features. set_features() is
  skipped entirely, so the update was lost outright.

Plain polling masked it: every eval calls _ensure_fresh_features(), which
re-runs load_features() → set_features() and flushes the pending value.
That is why this survived unnoticed.

Reproduced before the fix — after an SSE features event carrying
{"vips": ["u1"]}, the instance attribute held the new groups while
_global_ctx.saved_groups was still {}, and a $inGroup-gated feature
evaluated to False instead of True.

The fix

Adds set_saved_groups(), mirroring set_features(), which writes straight
through to the global context, and routes all four payload paths through it:
the HTTP load, the async load, the repository update callback and the SSE
handler. Adds get_saved_groups() for symmetry.

set_features() keeps its own _global_ctx.saved_groups assignment — now a
no-op in practice, but it still covers direct mutation of _saved_groups.

Testing

Two regression tests, both of which fail on the previous behavior:

• saved groups delivered by an SSE payload reach evaluation
• a refresh carrying savedGroups without features takes effect

787 tests pass, mypy clean, no new flake8 findings.

Note for reviewers: a caller-side set_saved_groups() is overwritten by the
next payload refresh, exactly as a caller-side set_features() would be. That
is existing, intended behavior — not changed here.

[greptile-apps]

Confidence Score: 4/5

The PR should not merge until saved groups are published without exposing a mixed-generation evaluation context during concurrent refreshes.

_{Reviews (1) · Last reviewed commit: "fix: sync saved groups to the evaluation..."}

[greptile-apps]

Mixed Payload Snapshots

During a payload refresh, this assignment updates the currently published global context before set_features() publishes the remaining payload state. Since evaluations read that context without locking and retain it by reference, a concurrent evaluation can combine new saved groups with features from the previous payload generation and return an incorrect feature value. Publish a replacement global context atomically under _payload_lock instead of mutating the active snapshot.

Prompt To Fix With AI

This is a comment left during a code review.
Path: growthbook/growthbook.py
Line: 1269

Comment:
**Mixed Payload Snapshots**

During a payload refresh, this assignment updates the currently published global context before `set_features()` publishes the remaining payload state. Since evaluations read that context without locking and retain it by reference, a concurrent evaluation can combine new saved groups with features from the previous payload generation and return an incorrect feature value. Publish a replacement global context atomically under `_payload_lock` instead of mutating the active snapshot.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[vazarkevych]

Correct, and it goes further than the race: this PR no longer fixes anything.

Main has since solved the original problem in _ingest_payload (3895482,
01ab957): savedGroups are assigned before set_features, map-only payloads
republish the context, the SSE path goes through the same helper, and writers
are serialized behind _payload_lock.

Verified both scenarios from this PR's regression tests against current main
without the patch — savedGroups from an SSE payload reach evaluation, and a
refresh carrying savedGroups without features takes effect. Both pass.

So the setter here would only add a second path doing the same job, and as you
point out it would do it by mutating the published snapshot — defeating the
atomic rebind in _publish_global_context that exists precisely to stop a
concurrent eval from pairing new saved groups with stale features.

Closing rather than reworking: taking the suggestion would mean reimplementing
_ingest_payload