Skip to content

feat: evaluate big segments#550

Open
beekld wants to merge 3 commits into
mainfrom
beeklimt/SDK-2366-2
Open

feat: evaluate big segments#550
beekld wants to merge 3 commits into
mainfrom
beeklimt/SDK-2366-2

Conversation

@beekld

@beekld beekld commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Evaluate big segments

Wires the big segment store wrapper into the server-side evaluator so flags referencing big segments actually evaluate, and annotates the reason with the big segments status. Builds on SDK-2366, which introduced the EvaluationReason::BigSegmentsStatus type.

What's in

  • Big segment matching in rules.cpp:
    • looks up membership for the unbounded context kind
    • tests the <key>.g<generation> segment ref
    • short-circuits to non-match on a store error
    • falls through to the segment's rules on a membership miss
  • EvaluationStack carries the per-evaluation status, membership cache, and per-key store-error set, so a context key is queried at most once per evaluation.
  • The aggregated status is stamped onto the returned EvaluationReason.

Design notes

When one evaluation queries multiple big segments with differing statuses, the least-trustworthy status wins (NOT_CONFIGURED > STORE_ERROR > STALE > HEALTHY). This follows the spec but diverges from Java/Go, which take the last status written.

Testing

Added unit tests covering each evaluation branch against an in-memory store.


Note

Medium Risk
Changes core segment-matching and evaluation-reason behavior for flags using big segments; incorrect membership or status aggregation could change targeting outcomes, though behavior is heavily unit-tested.

Overview
Big segment evaluation is wired into the server-side flag evaluator: unbounded segments now resolve membership via BigSegmentStoreWrapper instead of always returning non-match with a TODO.

EvaluationStack is extended with a non-owning store pointer, per-evaluation membership cache (one store lookup per context key), per-key store-error short-circuit, and aggregated BigSegmentsStatus using worst-wins precedence (NOT_CONFIGURED > STORE_ERROR > STALE > HEALTHY). Evaluator accepts an optional big-segment store, seeds the stack from it, and stamps the aggregated status onto the returned EvaluationReason when any big segment was consulted.

In Contains / MatchBigSegment, big segments use the unbounded context kind’s key, check <segmentKey>.g<generation>, treat store errors as non-match without retry, skip regular include/exclude lists, and fall through to segment rules on membership miss. New gtest coverage exercises not-configured, healthy/stale/error paths, precedence, caching, and rule fallthrough.

Reviewed by Cursor Bugbot for commit 9b37b9a. Bugbot is set up for automated code reviews on this repo. Configure here.

@beekld beekld requested a review from a team as a code owner June 10, 2026 04:55
@beekld beekld marked this pull request as draft June 10, 2026 04:58
@beekld beekld force-pushed the beeklimt/SDK-2366-2 branch 3 times, most recently from 1e33a1a to 20a23ce Compare June 10, 2026 19:52
@beekld beekld force-pushed the beeklimt/SDK-2366 branch from ae98702 to d0e181f Compare June 11, 2026 21:08
Base automatically changed from beeklimt/SDK-2366 to main June 11, 2026 21:51
@beekld beekld force-pushed the beeklimt/SDK-2366-2 branch from 20a23ce to c7a2817 Compare June 11, 2026 22:09
@beekld beekld marked this pull request as ready for review June 11, 2026 22:45
return detail;
}
EvaluationReason const& reason = *maybe_reason;
EvaluationReason updated(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Part of me wants this to be more colocated with the evaluation. But I suppose a real constructor change would be noticed.

Comment thread libs/server-sdk/src/evaluation/rules.cpp Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants