Skip to content

Commit 87bddbe

Browse files
committed
Tidy _classify_jsonpath: NamedTuple result, no None return
- Result type is now `JsonpathClassification(kind, trait_key)` — kind is a Literal of the five classification names; trait_key is set only when kind is "trait". - The "invalid jsonpath syntax" case is folded into the trait kind: a prop that doesn't parse as JSONPath becomes `("trait", prop)`, which is what the engine does when its jsonpath compile fails. Removes the None return entirely. - Translator always classifies — non-`$`-prefixed and empty-string props flow through the same code path; the `if classification is not None` guards drop out. Parity: 656 passed / 2 xfailed. Coverage: 100% line + branch.
1 parent 1cf1be7 commit 87bddbe

1 file changed

Lines changed: 49 additions & 39 deletions

File tree

src/flagsmith_sql_flag_engine/translator.py

Lines changed: 49 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131

3232
import json
3333
import re
34+
from typing import Literal, NamedTuple
3435

3536
import jsonpath_rfc9535
3637
from flag_engine.context.types import (
@@ -200,9 +201,21 @@ def _semver_sort_key_expr(ctx: TranslateContext, value_sql: str) -> str:
200201
# ---------------------------------------------------------------------------
201202

202203

203-
def _classify_jsonpath(
204-
prop: str,
205-
) -> tuple[str, ...] | None:
204+
JsonpathKind = Literal["identifier", "key", "trait", "untranslatable", "static"]
205+
206+
207+
class JsonpathClassification(NamedTuple):
208+
"""What a JSONPath property resolves to in the SQL setting.
209+
210+
`kind` selects the shape; `trait_key` carries the trait name only when
211+
`kind == "trait"`.
212+
"""
213+
214+
kind: JsonpathKind
215+
trait_key: str | None = None
216+
217+
218+
def _classify_jsonpath(prop: str) -> JsonpathClassification:
206219
"""Classify a JSONPath property by what it resolves to in the SQL setting.
207220
208221
Identity is per-row in our query model (each `IDENTITIES` row IS an
@@ -211,19 +224,14 @@ def _classify_jsonpath(
211224
to a row reference, not be statically pre-computed against the eval
212225
context.
213226
214-
Returns one of:
215-
- `("identifier",)` — `$.identity.identifier` (any syntax)
216-
- `("key",)` — `$.identity.key` (any syntax)
217-
- `("trait", "<key>")` — `$.identity.traits.<key>` (any syntax)
218-
- `("untranslatable",)` — under `$.identity` but doesn't map to row state
219-
- `("static",)` — non-identity (env, feature, etc.); pre-computable
220-
- `None` — not valid JSONPath syntax; caller falls back to
221-
a literal trait-key lookup, mirroring the engine
227+
A `prop` that doesn't parse as JSONPath is classified as a `("trait",
228+
prop)` — the engine treats unparseable `$.`-prefixed properties as
229+
literal trait keys, and we mirror that.
222230
"""
223231
try:
224232
compiled = jsonpath_rfc9535.compile(prop)
225233
except jsonpath_rfc9535.JSONPathSyntaxError:
226-
return None
234+
return JsonpathClassification("trait", prop)
227235
names: list[str] = []
228236
for s in compiled.segments:
229237
if len(s.selectors) != 1: # pragma: no cover - multi-selector segments not in dataset
@@ -238,19 +246,19 @@ def _classify_jsonpath(
238246
# `$.identity` — the whole identity object. Operators that
239247
# take a scalar fail-cast on a dict and the engine returns
240248
# False; static eval gives the same per-row answer.
241-
return ("static",)
249+
return JsonpathClassification("static")
242250
if len(names) == 2 and names[1] == "identifier":
243-
return ("identifier",)
251+
return JsonpathClassification("identifier")
244252
if len(names) == 2 and names[1] == "key":
245-
return ("key",)
253+
return JsonpathClassification("key")
246254
if len(names) == 3 and names[1] == "traits":
247-
return ("trait", names[2])
248-
return ("untranslatable",)
255+
return JsonpathClassification("trait", names[2])
256+
return JsonpathClassification("untranslatable")
249257
if names and names[0] == "identity":
250258
# Identity path with non-name selectors (wildcards, filters, etc.) —
251259
# we can't map those to fixed row references.
252-
return ("untranslatable",)
253-
return ("static",)
260+
return JsonpathClassification("untranslatable")
261+
return JsonpathClassification("static")
254262

255263

256264
def _engine_static_verdict(ctx: TranslateContext, cond: SegmentCondition) -> str:
@@ -361,16 +369,18 @@ def translate_condition(cond: SegmentCondition, ctx: TranslateContext) -> str |
361369
prop = cond.get("property") or ""
362370
val = cond.get("value")
363371

364-
# Classify any JSONPath-shaped property up front. Identity-bound paths
372+
# Classify the property up front. Identity-bound JSONPaths
365373
# (`$.identity.identifier`, `$.identity.key`, `$.identity.traits.<x>`) map
366-
# to row references; non-identity paths are eval-ctx-bound (constant for
367-
# every row) and get pre-computed; bad JSONPath syntax falls back to a
368-
# literal trait-key lookup, mirroring the engine.
369-
classification: tuple[str, ...] | None = None
370-
if prop.startswith("$"):
371-
classification = _classify_jsonpath(prop)
372-
if classification and classification[0] == "trait":
373-
prop = classification[1] # rewrite to bare trait key for downstream
374+
# to row references; non-identity JSONPaths are eval-ctx-bound (constant
375+
# for every row) and get pre-computed via the engine. Anything that
376+
# doesn't parse as JSONPath — including bare trait keys with no `$`
377+
# prefix and empty strings — comes back as `("trait", prop)`.
378+
classification = _classify_jsonpath(prop)
379+
if classification.kind == "trait":
380+
# Trait keys carried via `$.identity.traits.<x>` arrive normalised
381+
# to the bare key; literal trait keys come through untouched.
382+
assert classification.trait_key is not None
383+
prop = classification.trait_key
374384

375385
# PERCENTAGE_SPLIT — inline pure-SQL hash, no UDF.
376386
if op == "PERCENTAGE_SPLIT":
@@ -386,25 +396,26 @@ def translate_condition(cond: SegmentCondition, ctx: TranslateContext) -> str |
386396
return "FALSE"
387397
threshold = float(threshold_lit)
388398
identity: dict[str, object] = ctx.evaluation_context.get("identity") or {} # type: ignore[assignment]
399+
kind = classification.kind
389400
if not prop:
390401
# Implicit `$.identity.key` — engine returns False when no
391402
# identity, or when the identity lacks `key` (the engine never
392403
# synthesises one from env+identifier).
393404
if not identity.get("key"):
394405
return "FALSE"
395406
value_expr = ctx.dialect.cast_string(ctx.identity_key_expr)
396-
elif classification == ("key",):
407+
elif kind == "key":
397408
if not identity.get("key"):
398409
return "FALSE"
399410
value_expr = ctx.dialect.cast_string(ctx.jsonpath_expr("$.identity.key"))
400-
elif classification == ("identifier",):
411+
elif kind == "identifier":
401412
if not identity.get("identifier"):
402413
return "FALSE"
403414
value_expr = ctx.dialect.cast_string(ctx.jsonpath_expr("$.identity.identifier"))
404-
elif classification == ("untranslatable",):
415+
elif kind == "untranslatable":
405416
# `$.identity.<X>` we don't represent in the row schema.
406417
return None
407-
elif classification == ("static",):
418+
elif kind == "static":
408419
# Non-identity JSONPath: the engine hashes the resolved value.
409420
# We'd need to bake it as a literal hash subject — leave for
410421
# future work and let the caller fall back to the engine.
@@ -423,24 +434,23 @@ def translate_condition(cond: SegmentCondition, ctx: TranslateContext) -> str |
423434
# nothing, the comparator's cast fails, returns False.
424435
return "FALSE"
425436

426-
if classification in (("identifier",), ("key",)):
437+
if classification.kind in ("identifier", "key"):
427438
path = ctx.jsonpath_expr(
428-
"$.identity.identifier" if classification == ("identifier",) else "$.identity.key"
439+
"$.identity.identifier" if classification.kind == "identifier" else "$.identity.key"
429440
)
430441
if op == "IS_SET":
431442
return "TRUE"
432443
if op == "IS_NOT_SET":
433444
return "FALSE"
434445
return _comparison(ctx, op, path, val, is_jsonpath=True)
435-
if classification == ("untranslatable",):
446+
if classification.kind == "untranslatable":
436447
# Identity-bound JSONPath we can't map to row state — caller falls
437448
# back to the engine.
438449
return None
439-
if classification == ("static",):
450+
if classification.kind == "static":
440451
return _engine_static_verdict(ctx, cond)
441-
# Either no JSONPath prefix, or `prop` started with `$` but JSONPath
442-
# syntax parsing failed — engine treats the latter as a literal trait
443-
# key. Fall through to the trait branch.
452+
# `kind == "trait"` — fall through to the trait branch below with
453+
# `prop` already rewritten to the bare trait key.
444454

445455
# Trait-bound predicates: the dialect knows how to extract traits from
446456
# whatever shape it stores them in (Snowflake's VARIANT column today,

0 commit comments

Comments
 (0)