Repository navigation
fix(flags): canonicalize numeric group keys for local bucketing - #1071
Kayvan-Zahiri wants to merge 2 commits into
Conversation
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
…g-key-bucketing # Conflicts: # posthog/test/test_feature_flags.py
A quick word from the signature departmentHey @Kayvan-Zahiri, thanks for the PR! GitHub couldn't verify the signature on this commit:
PostHog requires verified commit signatures before merging. So we've built a bot to do the nagging. You're talking to it. Let's get this verifiedIf signing isn't set up yet, follow our signing guide or GitHub's guide. Already signed? Make sure GitHub recognizes your key. SSH keys must be registered as a Signing Key, not only as an Authentication Key. Once signing is set up, if the unsigned commit is your latest commit, run: git commit --amend --no-edit -S
git push --force-with-leaseThose commands fix only your latest commit. Older unsigned commits need signing too. Coordinate with anyone else using the branch before rewriting its history. Save your future self another visit from me: git config --local commit.gpgsign trueThat enables signing for future commits; it doesn't sign existing ones. A I'll check again after your next push and update this comment. |
Motivation and context
Numeric group identifiers currently reach the local rollout hash through Python string interpolation. For a group flag called
group-betaat 50% rollout, group key0.00001hashes as1e-05, returning false, while its canonical string key"0.00001"returns true. Invalid group keys such as booleans or containers can also return a definitive local result instead of leaving remote evaluation eligible.Normalize group keys before bucketing in both legacy flag-level aggregation and mixed condition-level aggregation. Strings stay unquoted, finite numbers use the existing backend-compatible JSON rendering, and unsupported values raise the evaluator's existing inconclusive signal. No public method, option, or type is added.
This follows the published group-key bucketing requirement.
Verification
The new tests use the public
Client.get_feature_flag()andClient.evaluate_flags()APIs with locally loaded definitions. They cover legacy/mixed aggregation, finite numeric/string controls, unsupported boolean/null/container/non-finite values, and remote fallback. Before the fix there are 16 failing subtests; after it all pass.Focused flag suites: 293 passed, 37 subtests passed. Full SDK suite: 4,620 passed, 16 skipped, 78 subtests passed (134 warnings). Repository-wide Ruff formatting/lint, mypy across 300 source files, and the public API snapshot check pass. The local SDK was exercised against its own evaluator and mocked remote responses; no hosted production flag configuration was changed.
A patch changeset is included. No public API snapshot changes are expected.
Agent context
Autonomy: Fully autonomous
Codex independently selected and reproduced this defect while exercising the Python SDK, implemented the scoped fix, and ran regression and quality checks. A human authorized contribution work but has not reviewed this implementation. The PR is a draft and requires human review; it will not be auto-approved or merged.