fix(features): preserve multivariate bucketing salt across feature state recreation#7914
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a nullable Estimated code review effort: 3 (Moderate) | ~25 minutes Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7914 +/- ##
========================================
Coverage 98.63% 98.63%
========================================
Files 1496 1497 +1
Lines 59072 59261 +189
========================================
+ Hits 58266 58455 +189
Misses 806 806 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ate recreation Multivariate variant bucketing is salted on the feature state id, so any flow that recreates a feature state (publishing a new v2 version, editing multivariate weights) changed the salt and re-bucketed already-enrolled identities. Add an mv_hashing_salt field that get_multivariate_feature_state_value seeds on (falling back to the id), preserved across clone() so recreation keeps the original seed. The engine mapper feeds the salt through django_id, so edge and local evaluation stay stable without an engine document schema change. Also add a BEFORE_CREATE guard that raises when a v2 feature state is recreated outside clone() (the previous version already had it but the new row carries no salt), to catch future regressions in tests. Closes #7913
71ba4ee to
0822078
Compare
… guard to segment overrides
0822078 to
a32e197
Compare
Docker builds report
|
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Failed testsfirefox › tests/environment-permission-test.pw.ts › Environment Permission Tests › Environment-level permissions control access to features, identities, and segments @enterprise Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
|
Visual Regression19 screenshots compared. See report for details. |
|
@CodeRabbit review |
✅ Action performedReview finished.
|
Zaimwa9
left a comment
There was a problem hiding this comment.
Ok great, good call to have pushed back and rethink the implementation.
I couldn't find flaws in it and it simplified a lot the mental model. Approving with one nit that shouldn't happen normally so feel free to resolve and ignore
docs/if required so people know about the feature.Changes
Closes #7913
Multivariate bucketing is salted on the feature state id, so any flow that
recreates a feature state (version publish, weight edits, change request
commits) re-bucketed already-enrolled identities.
FeatureState.mv_hashing_salt(nullable, internal — not exposed via APIor audit logs). Bucketing seeds on the new
mv_hashing_seedproperty(
salt or id); no backfill needed.BEFORE_CREATEhook makes any new multivariate feature state inherit theseed from the live state it supersedes, so every creation path (change
request drafts, direct recreates, version edits) keeps bucketing stable
without knowing the salt exists.
clone()copies it like any other field.django_id, so Core, Edge and local-evalSDKs bucket identically with no document schema change.
Known gap: v1 change request drafts already open when this deploys predate the
salt and will re-bucket once when committed — that is today's behaviour, one
last time; not worth a backfill migration.
How did you test this code?
Unit tests: seed fallback, clone preservation (50 identities keep their
variant), engine mapper output, seed inheritance for v1/v2 direct recreates
and fresh lineages, and change request commits keeping variant assignment
stable end to end.