diff --git a/src/flagsmith_sql_flag_engine/translator.py b/src/flagsmith_sql_flag_engine/translator.py index a49ff87..1ba73f0 100644 --- a/src/flagsmith_sql_flag_engine/translator.py +++ b/src/flagsmith_sql_flag_engine/translator.py @@ -432,7 +432,6 @@ def translate_condition(cond: SegmentCondition, ctx: TranslateContext) -> str | # Engine: float() on the threshold raises → returns False. return "FALSE" threshold = float(threshold_lit) - identity: dict[str, object] = ctx.evaluation_context.get("identity") or {} # type: ignore[assignment] kind = classification.kind if not prop: # In traditional engine implementations, this branch implies @@ -457,13 +456,18 @@ def translate_condition(cond: SegmentCondition, ctx: TranslateContext) -> str | # future work and let the caller fall back to the engine. return None else: - # Plain trait key, or `$.identity.traits.` rewritten to - # the bare key. Hash subject pulls from `i.traits:""` - # per row. - traits = identity.get("traits") or {} - if not isinstance(traits, dict) or prop not in traits: - return "FALSE" - value_expr = ctx.dialect.cast_string(ctx.trait_path(prop)) + # Plain trait key, or `$.identity.traits.` rewritten to the + # bare key. Hash the per-row trait value from `i.traits:""`. + # A row missing the trait has no value to bucket, so it cannot + # match — mirroring the engine's `context_value is None -> False`. + trait_sql = ctx.trait_path(prop) + split = _percentage_split_expr( + ctx, + ctx.segment_key, + ctx.dialect.cast_string(trait_sql), + threshold, + ) + return f"({trait_sql} IS NOT NULL AND {split})" return _percentage_split_expr(ctx, ctx.segment_key, value_expr, threshold) if not prop: diff --git a/tests/test_translator_unit.py b/tests/test_translator_unit.py index df12a6c..119259b 100644 --- a/tests/test_translator_unit.py +++ b/tests/test_translator_unit.py @@ -16,13 +16,6 @@ def _ctx(env_key: str = "test-env-key", env_name: str = "Test") -> TranslateContext: eval_ctx: EvaluationContext = { "environment": {"key": env_key, "name": env_name}, - # PERCENTAGE_SPLIT short-circuits to FALSE when the prop isn't in - # the eval context's traits, so seed the traits referenced below. - "identity": { - "identifier": "u", - "key": "k", - "traits": {"plan": "growth", "country": "GB", "uuid_attr": "abc"}, - }, } return TranslateContext(evaluation_context=eval_ctx, dialect=ClickHouseDialect()) @@ -164,6 +157,31 @@ def test_translate_segment__percentage_split_on_trait__uses_trait_subcolumn() -> assert "MD5(" in sql +def test_translate_segment__percentage_split_on_trait__guards_missing_trait_per_row() -> None: + # Given + seg: SegmentContext = { + "key": "101", + "name": "s", + "rules": [ + { + "type": "ALL", + "conditions": [{"operator": "PERCENTAGE_SPLIT", "property": "plan", "value": "10"}], + } + ], + } + + # When + sql = translate_segment(seg, _ctx()) + + # Then + assert sql is not None + assert "FALSE" not in sql + assert "i.traits.`plan`" in sql + assert "IS NOT NULL" in sql + assert "MD5(" in sql + assert "<= 10.0" in sql + + def test_translate_segment__jsonpath_identity_identifier__uses_column_directly() -> None: # Given a condition referencing $.identity.identifier seg: SegmentContext = { @@ -623,25 +641,6 @@ def test_translate_segment__percentage_split_unknown_jsonpath__returns_none() -> assert translate_segment(seg, _ctx()) is None -def test_translate_segment__percentage_split_trait_not_in_context__compiles_to_false() -> None: - # Given a PERCENTAGE_SPLIT on a trait the eval context's identity doesn't carry - seg: SegmentContext = { - "key": "ps6", - "name": "s", - "rules": [ - { - "type": "ALL", - "conditions": [ - {"operator": "PERCENTAGE_SPLIT", "property": "missing_trait", "value": "50"} - ], - } - ], - } - - # When / Then the predicate collapses to FALSE - assert translate_segment(seg, _ctx()) == "((FALSE))" - - def test_translate_segment__is_set_on_identity_identifier__emits_true() -> None: # Given an IS_SET on $.identity.identifier — every IDENTITIES row IS an # identity, so the predicate is unconditionally true diff --git a/uv.lock b/uv.lock index a04036b..6dc0010 100644 --- a/uv.lock +++ b/uv.lock @@ -286,7 +286,7 @@ wheels = [ [[package]] name = "flagsmith-sql-flag-engine" -version = "0.1.0a2" +version = "0.1.2" source = { editable = "." } dependencies = [ { name = "flagsmith-flag-engine" },