Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change adds services that evaluate identity and environment feature states through flag-engine and pair each result with its source feature state. API views, serializers and integration wrappers now consume these evaluated states. The change also moves evaluation-context mapping into the evaluation module, adds split-weight parsing, and updates tests for evaluation precedence, segment priority and multivariate allocation. Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: ⚪ Minimal · up to The change preserves override selection, strengthens the allocation check, and excludes the synthetic override context from reported segment membership. No actionable merge risk remains. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## stack/1-evaluation-mappers #8574 +/- ##
===========================================================
Coverage 98.81% 98.81%
===========================================================
Files 1644 1647 +3
Lines 67435 67524 +89
===========================================================
+ Hits 66636 66724 +88
- Misses 799 800 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
01b53c5 to
ed92679
Compare
ed92679 to
882de75
Compare
882de75 to
c2846fc
Compare
c2846fc to
e26a8b5
Compare
e26a8b5 to
8118f64
Compare
8118f64 to
3b83433
Compare
3b83433 to
c47236f
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 4c6bc876-a540-4f11-a0ed-4b5bffcaac95
📒 Files selected for processing (41)
api/environments/identities/models.pyapi/environments/identities/views.pyapi/environments/sdk/serializers.pyapi/evaluation/mappers.pyapi/evaluation/results.pyapi/evaluation/services.pyapi/evaluation/types.pyapi/features/models.pyapi/features/serializers.pyapi/features/views.pyapi/integrations/amplitude/amplitude.pyapi/integrations/common/wrapper.pyapi/integrations/heap/heap.pyapi/integrations/mixpanel/mixpanel.pyapi/integrations/rudderstack/rudderstack.pyapi/integrations/segment/segment.pyapi/integrations/webhook/serializers.pyapi/integrations/webhook/webhook.pyapi/tests/conftest.pyapi/tests/integration/conftest.pyapi/tests/integration/environments/identities/test_integration_identities.pyapi/tests/types.pyapi/tests/unit/environments/identities/test_unit_identities_models.pyapi/tests/unit/environments/identities/test_unit_identities_views.pyapi/tests/unit/evaluation/test_unit_evaluation_mappers.pyapi/tests/unit/evaluation/test_unit_evaluation_services.pyapi/tests/unit/experimentation/test_services.pyapi/tests/unit/features/test_unit_features_models.pyapi/tests/unit/features/versioning/test_unit_versioning_versioning_service.pyapi/tests/unit/features/workflows/core/test_unit_workflows_models.pyapi/tests/unit/import_export/test_unit_import_export_export.pyapi/tests/unit/integrations/amplitude/test_unit_amplitude.pyapi/tests/unit/integrations/heap/test_unit_heap.pyapi/tests/unit/integrations/mixpanel/test_unit_mixpanel.pyapi/tests/unit/integrations/rudderstack/test_unit_rudderstack.pyapi/tests/unit/integrations/segment/test_unit_segment.pyapi/tests/unit/integrations/test_unit_integration.pyapi/tests/unit/integrations/webhook/test_unit_webhook.pyapi/tests/unit/integrations/webhook/test_unit_webhook_serializers.pyapi/tests/unit/util/mappers/test_unit_mappers_engine.pyapi/util/mappers/engine.py
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
791a14c to
7a94e1b
Compare
7a94e1b to
5ad2f43
Compare
5ad2f43 to
69b7b83
Compare
Feature states now only ever hold stored values. The evaluation services return each engine result paired with the feature state it came from, and evaluated-path serialisers read evaluated fields from the former and stored fields from the latter. Environment flags are evaluated too, without segments, so their output is unchanged.
… overrides The identity overrides pseudo-segment always matches, so every segment was reported as a member for an identity with overrides. Evaluating once per payload also stops the query count from growing with the number of segments. Also tighten the rollout split assertion, and cover the analytics integrations reporting evaluated rather than stored values.
69b7b83 to
dd18806
Compare
The engine only splits between variants for an identity, so environment flags no longer query them.
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to #6654
In this PR, we route all identity flag evaluations (SDK API + dashboard segment membership checks) via the evaluation engine. Environment flags go through it too, without segments for now, so their output is unchanged.
Evaluation services now return
EvaluatedFeatureState, pairing each engine result with the feature state it came from. SDK serialisers, the identity webhook and the analytics integrations read evaluated fields (enabled, value,variant) from the former, and stored fields from the latter.FeatureStateno longer evaluates:get_feature_state_value()returns the stored value only.The dashboard
…/featurestates/all/view evaluates through the engine as well, but its serialiser is shared with edge identities, so it switches toEvaluatedFeatureStatein #8575.No behaviour change is intended, except that
?feature=is now case-sensitive, as in Edge API. Multivariate allocation behaviour is pinned to static expectations in tests, and verified to be identical to current Core.There are some query count / performance implications:
Identity.get_segmentsnow fetches feature states.GET /flags/?identifier=…&feature=…now fetches all feature states regardless of thefeaturequery parameter, because dependent flags.GET /identities/{identifier}/asks for flags and segments separately, so it builds a context twice.GET /flags/makes one more query, prefetching multivariate values.percentage_allocationnow comes from the engine's split weight, rather than a query per flag.How did you test this code?
Multivariate bucketing is pinned to static data in
test_evaluate_identity__multivariate_feature__buckets_as_before_the_engine. The feature is split into ten equal variants, so the variant an identity gets tells you which decile its hash fell in. The environment API key and hashing salt are pinned too. The expected values were derived from Core API's allocation before the engine took over.The before-and-after tests for bucketing stability (change request commit, versioning weight change, experiment rollout re-applied) now go through
evaluate_identityvia avariant_assignmentfixture.The identity integration tests no longer mock the engine's hashing.
The SDK view tests assert full responses; only the deprecated endpoint's query counts change. The OpenAPI schema is unchanged.
Added a test for
varianton a multivariate flag with no variants, or a keyless one; it passes onmaintoo.