Skip to content

Commit 31a4958

Browse files
authored
fix(translator): % Split over a trait folds to FALSE without an identity context (#18)
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
1 parent 26ad3d5 commit 31a4958

3 files changed

Lines changed: 38 additions & 35 deletions

File tree

src/flagsmith_sql_flag_engine/translator.py

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,6 @@ def translate_condition(cond: SegmentCondition, ctx: TranslateContext) -> str |
432432
# Engine: float() on the threshold raises → returns False.
433433
return "FALSE"
434434
threshold = float(threshold_lit)
435-
identity: dict[str, object] = ctx.evaluation_context.get("identity") or {} # type: ignore[assignment]
436435
kind = classification.kind
437436
if not prop:
438437
# In traditional engine implementations, this branch implies
@@ -457,13 +456,18 @@ def translate_condition(cond: SegmentCondition, ctx: TranslateContext) -> str |
457456
# future work and let the caller fall back to the engine.
458457
return None
459458
else:
460-
# Plain trait key, or `$.identity.traits.<X>` rewritten to
461-
# the bare key. Hash subject pulls from `i.traits:"<key>"`
462-
# per row.
463-
traits = identity.get("traits") or {}
464-
if not isinstance(traits, dict) or prop not in traits:
465-
return "FALSE"
466-
value_expr = ctx.dialect.cast_string(ctx.trait_path(prop))
459+
# Plain trait key, or `$.identity.traits.<X>` rewritten to the
460+
# bare key. Hash the per-row trait value from `i.traits:"<key>"`.
461+
# A row missing the trait has no value to bucket, so it cannot
462+
# match — mirroring the engine's `context_value is None -> False`.
463+
trait_sql = ctx.trait_path(prop)
464+
split = _percentage_split_expr(
465+
ctx,
466+
ctx.segment_key,
467+
ctx.dialect.cast_string(trait_sql),
468+
threshold,
469+
)
470+
return f"({trait_sql} IS NOT NULL AND {split})"
467471
return _percentage_split_expr(ctx, ctx.segment_key, value_expr, threshold)
468472

469473
if not prop:

tests/test_translator_unit.py

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,6 @@
1616
def _ctx(env_key: str = "test-env-key", env_name: str = "Test") -> TranslateContext:
1717
eval_ctx: EvaluationContext = {
1818
"environment": {"key": env_key, "name": env_name},
19-
# PERCENTAGE_SPLIT short-circuits to FALSE when the prop isn't in
20-
# the eval context's traits, so seed the traits referenced below.
21-
"identity": {
22-
"identifier": "u",
23-
"key": "k",
24-
"traits": {"plan": "growth", "country": "GB", "uuid_attr": "abc"},
25-
},
2619
}
2720
return TranslateContext(evaluation_context=eval_ctx, dialect=ClickHouseDialect())
2821

@@ -164,6 +157,31 @@ def test_translate_segment__percentage_split_on_trait__uses_trait_subcolumn() ->
164157
assert "MD5(" in sql
165158

166159

160+
def test_translate_segment__percentage_split_on_trait__guards_missing_trait_per_row() -> None:
161+
# Given
162+
seg: SegmentContext = {
163+
"key": "101",
164+
"name": "s",
165+
"rules": [
166+
{
167+
"type": "ALL",
168+
"conditions": [{"operator": "PERCENTAGE_SPLIT", "property": "plan", "value": "10"}],
169+
}
170+
],
171+
}
172+
173+
# When
174+
sql = translate_segment(seg, _ctx())
175+
176+
# Then
177+
assert sql is not None
178+
assert "FALSE" not in sql
179+
assert "i.traits.`plan`" in sql
180+
assert "IS NOT NULL" in sql
181+
assert "MD5(" in sql
182+
assert "<= 10.0" in sql
183+
184+
167185
def test_translate_segment__jsonpath_identity_identifier__uses_column_directly() -> None:
168186
# Given a condition referencing $.identity.identifier
169187
seg: SegmentContext = {
@@ -623,25 +641,6 @@ def test_translate_segment__percentage_split_unknown_jsonpath__returns_none() ->
623641
assert translate_segment(seg, _ctx()) is None
624642

625643

626-
def test_translate_segment__percentage_split_trait_not_in_context__compiles_to_false() -> None:
627-
# Given a PERCENTAGE_SPLIT on a trait the eval context's identity doesn't carry
628-
seg: SegmentContext = {
629-
"key": "ps6",
630-
"name": "s",
631-
"rules": [
632-
{
633-
"type": "ALL",
634-
"conditions": [
635-
{"operator": "PERCENTAGE_SPLIT", "property": "missing_trait", "value": "50"}
636-
],
637-
}
638-
],
639-
}
640-
641-
# When / Then the predicate collapses to FALSE
642-
assert translate_segment(seg, _ctx()) == "((FALSE))"
643-
644-
645644
def test_translate_segment__is_set_on_identity_identifier__emits_true() -> None:
646645
# Given an IS_SET on $.identity.identifier — every IDENTITIES row IS an
647646
# identity, so the predicate is unconditionally true

uv.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)