Skip to content

Commit 0e23b2e

Browse files
committed
fix(translator): Hash identity-key percentage splits without a context identity
A PERCENTAGE_SPLIT keyed on the identity (implicit `$.identity.key`, `$.identity.key`, or `$.identity.identifier`) compiled to FALSE when the evaluation context carried no identity. But the translated predicate runs per-row over IDENTITIES, where the identity-key and identifier columns are always present, so it should hash the row column regardless of whether the row-oriented context supplies an identity. The guard made row-oriented callers (segment-membership counts/members) see every percentage-split segment as empty. Drop the eval-context identity guard for those three cases and emit the per-row hash. Trait-keyed splits are unchanged. No engine-test-data case translates a percentage split with an identity-less context, so the SQL for every parity case is byte-identical (the guard was always passing there) and no _XFAIL_CASE_NAMES entries are needed. This does intentionally diverge from flag_engine's single-eval "no identity -> False" for the identity-less case, which is correct for the row-oriented SQL engine where every row is an identity. beep boop Claude-Session: https://claude.ai/code/session_01EgZ5iHpDASZzCapiHRxHLB
1 parent cfc3d4e commit 0e23b2e

2 files changed

Lines changed: 32 additions & 24 deletions

File tree

src/flagsmith_sql_flag_engine/translator.py

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -435,19 +435,14 @@ def translate_condition(cond: SegmentCondition, ctx: TranslateContext) -> str |
435435
identity: dict[str, object] = ctx.evaluation_context.get("identity") or {} # type: ignore[assignment]
436436
kind = classification.kind
437437
if not prop:
438-
# Implicit `$.identity.key` — engine returns False when no
439-
# identity, or when the identity lacks `key`. The engine
440-
# never synthesises one from env+identifier.
441-
if not identity.get("key"):
442-
return "FALSE"
438+
# Implicit `$.identity.key`. The key is always present in the store,
439+
# so the split is translatable whether or not the evaluation context
440+
# carries an identity. This intentionally diverges from the engine's
441+
# "no identity → False" verdict.
443442
value_expr = ctx.dialect.cast_string(ctx.identity_key_expr)
444443
elif kind == "key":
445-
if not identity.get("key"):
446-
return "FALSE"
447444
value_expr = ctx.dialect.cast_string(ctx.jsonpath_expr("$.identity.key"))
448445
elif kind == "identifier":
449-
if not identity.get("identifier"):
450-
return "FALSE"
451446
value_expr = ctx.dialect.cast_string(ctx.jsonpath_expr("$.identity.identifier"))
452447
elif kind == "identity_object":
453448
# PERCENTAGE_SPLIT on `$.identity` — the whole dict. Engine

tests/test_translator_unit.py

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -542,9 +542,10 @@ def _ctx_identity_without(field: str) -> TranslateContext:
542542
)
543543

544544

545-
def test_translate_segment__percentage_split_implicit_key_no_identity__compiles_to_false() -> None:
546-
# Given a PERCENTAGE_SPLIT with no property (implicit `$.identity.key`) and an eval
547-
# context with no identity
545+
def test_translate_segment__percentage_split_implicit_key_no_identity__inlines_md5() -> None:
546+
# Given a PERCENTAGE_SPLIT with no property (implicit `$.identity.key`) and an
547+
# eval context with no identity — the row-oriented case (e.g. segment-membership
548+
# counting over the whole IDENTITIES table)
548549
seg: SegmentContext = {
549550
"key": "ps2",
550551
"name": "s",
@@ -556,12 +557,19 @@ def test_translate_segment__percentage_split_implicit_key_no_identity__compiles_
556557
],
557558
}
558559

559-
# When / Then the predicate collapses to FALSE — engine returns False without identity
560-
assert translate_segment(seg, _ctx_no_identity()) == "((FALSE))"
560+
# When translated
561+
sql = translate_segment(seg, _ctx_no_identity())
561562

563+
# Then it hashes the per-row identity-key column rather than folding to
564+
# FALSE — the row supplies the subject, so a context identity is not needed
565+
assert sql is not None
566+
assert "FALSE" not in sql
567+
assert "MD5(" in sql
568+
assert "<= 50.0" in sql
562569

563-
def test_translate_segment__percentage_split_identity_key_missing__compiles_to_false() -> None:
564-
# Given the eval context's identity has no `key`
570+
571+
def test_translate_segment__percentage_split_key_no_context_identity__inlines_md5() -> None:
572+
# Given a PERCENTAGE_SPLIT on `$.identity.key` and a context identity without `key`
565573
seg: SegmentContext = {
566574
"key": "ps3",
567575
"name": "s",
@@ -575,14 +583,16 @@ def test_translate_segment__percentage_split_identity_key_missing__compiles_to_f
575583
],
576584
}
577585

578-
# When / Then the predicate collapses to FALSE
579-
assert translate_segment(seg, _ctx_identity_without("key")) == "((FALSE))"
586+
# When / Then it still hashes the per-row identity-key column
587+
sql = translate_segment(seg, _ctx_identity_without("key"))
588+
assert sql is not None
589+
assert "FALSE" not in sql
590+
assert "MD5(" in sql
580591

581592

582-
def test_translate_segment__percentage_split_identity_identifier_missing__compiles_to_false() -> (
583-
None
584-
):
585-
# Given the eval context's identity has no `identifier`
593+
def test_translate_segment__percentage_split_identifier_no_context_identity__inlines_md5() -> None:
594+
# Given a PERCENTAGE_SPLIT on `$.identity.identifier` and a context identity
595+
# without `identifier`
586596
seg: SegmentContext = {
587597
"key": "ps4",
588598
"name": "s",
@@ -600,8 +610,11 @@ def test_translate_segment__percentage_split_identity_identifier_missing__compil
600610
],
601611
}
602612

603-
# When / Then the predicate collapses to FALSE
604-
assert translate_segment(seg, _ctx_identity_without("identifier")) == "((FALSE))"
613+
# When / Then it still hashes the per-row identifier column
614+
sql = translate_segment(seg, _ctx_identity_without("identifier"))
615+
assert sql is not None
616+
assert "FALSE" not in sql
617+
assert "MD5(" in sql
605618

606619

607620
def test_translate_segment__percentage_split_unknown_jsonpath__returns_none() -> None:

0 commit comments

Comments
 (0)