fix(flags): do not match empty OR cohorts - #1004
Draft
Kayvan-Zahiri wants to merge 1 commit into
Draft
Kayvan-Zahiri wants to merge 1 commit into
Kayvan-Zahiri wants to merge 1 commit into
Conversation
Use the group identity for empty cohort filters so local evaluation agrees with the published SDK contract. Cover nested groups and single/bulk membership evaluation.
This branch has not been deployed
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.
💡 Motivation and Context
An empty
ORcohort currently matches every user during local flag evaluation. A flag targeting that cohort is therefore enabled, while anot_incondition incorrectly excludes everyone. This also affects emptyORgroups nested inside other cohort groups.Return the group's Boolean identity when its
valueslist is empty:falseforOR,trueforAND. The canonical empty object{}remains a match. This follows the published local evaluator contract for empty cohort groups; it changes no public API surface. The earlier cohort fix #836 preserved empty groups but did not cover emptyORsemantics.💚 How did you test it?
in/not_incohort membership through single and bulk flag evaluation. They now pass, and the public API tests assert no remote fallback.PYTHONPATH=$PWD .venv/bin/python -m pytest --verbose --timeout=30). The explicit import path is needed on this macOS checkout because its editable-install.pthis marked hidden and ignored by subprocess interpreters.git diff --checkall pass.📝 Checklist
If releasing new changes
sampo addto generate a changeset file.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Kayvan-Zahiri is the DRI. GitHub rejected the self-assignment update for this external contribution; maintainer assignment is needed.
Codex identified the contract mismatch, wrote and ran the regressions, and prepared this change using shell tools, pytest, and the GitHub CLI. A second Codex agent reviewed the diff. The development session is private; no independent human code review is claimed. This draft is ready for maintainer review.