Skip to content

fix(translator): % Split over a trait folds to FALSE without an identity context#18

Merged
khvn26 merged 1 commit into
mainfrom
fix/percentage-split-over-trait
Jul 1, 2026
Merged

fix(translator): % Split over a trait folds to FALSE without an identity context#18
khvn26 merged 1 commit into
mainfrom
fix/percentage-split-over-trait

Conversation

@khvn26

@khvn26 khvn26 commented Jul 1, 2026

Copy link
Copy Markdown
Member

Contributes to Flagsmith/flagsmith#7934.

#16 fixed PERCENTAGE_SPLIT on the identity key/identifier folding to FALSE in an identity-less context; the trait branch had the same guard and was missed.

Testing

  • Added a translator unit test asserting a trait PERCENTAGE_SPLIT with no identity in context hashes the per-row subcolumn (guarded by IS NOT NULL) instead of folding to FALSE.
  • Dropped the identity-trait seeding from the shared unit-test context (it only existed to work around this guard) and removed the test that asserted the old FALSE behaviour.

…ity context

The trait branch of PERCENTAGE_SPLIT checked the eval context's
identity.traits and returned FALSE when the property was absent, instead of
hashing the per-row traits subcolumn. In an identity-less context (segment
membership counts/list) that dict is empty, so every trait-keyed split
compiled to FALSE. Hash the per-row value guarded by IS NOT NULL, mirroring
the engine's context_value-is-None -> False, per row.

beep boop
Claude-Session: https://claude.ai/code/session_01EgZ5iHpDASZzCapiHRxHLB
@khvn26 khvn26 requested a review from a team as a code owner July 1, 2026 18:39
@khvn26 khvn26 requested review from emyller and removed request for a team July 1, 2026 18:39
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

File Coverage Missing
All files 100%

Minimum allowed coverage is 100%

Generated by 🐒 cobertura-action against be79c72

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the SQL translation logic for PERCENTAGE_SPLIT conditions on traits. Instead of statically short-circuiting to FALSE when a trait is missing from the evaluation context, the translator now generates SQL that dynamically checks if the trait is not null per row before applying the percentage split expression. The unit tests have been updated to reflect this change, and the package version in uv.lock has been bumped to 0.1.2. I have no feedback to provide.

@emyller emyller left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with a non-blocking suggestion

Comment thread tests/test_translator_unit.py
@khvn26 khvn26 merged commit 31a4958 into main Jul 1, 2026
3 checks passed
@emyller emyller deleted the fix/percentage-split-over-trait branch July 1, 2026 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants