WIP: fix(features): preserve multivariate bucketing salt across feature state recreation#7914
Draft
gagantrivedi wants to merge 1 commit into
Draft
WIP: fix(features): preserve multivariate bucketing salt across feature state recreation#7914gagantrivedi wants to merge 1 commit into
gagantrivedi wants to merge 1 commit into
Conversation
…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
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Closes #7913
Multivariate variant bucketing is salted on the feature state id
(
FeatureState.get_multivariate_feature_state_valuehashes[self.id, identity_hash_key]).Any flow that recreates a feature state — publishing a new version or editing
multivariate weights under v2 versioning — produced a new id, which changed the
salt and re-bucketed already-enrolled identities (the regression behind the
experiment rollout reshuffle in #7912).
This adds a stable seed that survives recreation:
FeatureState.mv_hashing_salt(nullableIntegerField, migration0067).get_multivariate_feature_state_valuenow seeds onself.mv_hashing_salt or self.id.clone()setsclone.mv_hashing_salt = self.mv_hashing_salt or self.id,capturing the source id the first time and carrying it down every subsequent
clone. No backfill needed: existing rows fall back to their id (buckets don't
move on deploy); the value only materialises at the moment of a clone — exactly
when the id would otherwise drift.
django_id = feature_state.mv_hashing_salt or feature_state.pk,so the engine document, the context-mapper key, and local-eval SDKs all bucket
on the salt — no engine model or environment document schema change.
BEFORE_CREATEhook raises if a v2 feature state is recreatedoutside
clone()(the previous version already had a state for thisenvironment/feature/segment but the new row carries no salt). Identity
overrides are skipped (they are never versioned or cloned). This is a tripwire
to catch future code paths that bypass
clone().An audit confirmed
clone()is currently the only path that recreates anexisting feature state — every direct
FeatureState.objects.createis agenuinely new lineage — so the chokepoint assumption holds today.
How did you test this code?
Unit tests added/updated in
tests/unit/features/test_unit_features_models.pyand
tests/unit/util/mappers/test_unit_mappers_engine.py:django_id;new override.
make lint,make typecheckand the affected unit/integration test files passlocally. Full suite run is in progress.