diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1dc03ec..467eb78 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -37,3 +37,17 @@ jobs: - name: Test with pytest run: | pytest + + corpus-freshness: + # Drift check between tests/cases.json and the JS SDK's cases.json. + # Fails the build on any "missing" case not listed in + # tests/scripts/corpus_skiplist.json. + # Extras (Python's local additions) are reported but never fail. + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.13" + - name: Check corpus freshness vs JS SDK + run: python3 tests/scripts/check_corpus_freshness.py diff --git a/growthbook/core.py b/growthbook/core.py index 1b4e72d..e7f5657 100644 --- a/growthbook/core.py +++ b/growthbook/core.py @@ -1,4 +1,5 @@ import logging +import math import re import json from functools import lru_cache @@ -106,6 +107,15 @@ def elemMatch(condition, attributeValue, savedGroups) -> bool: return False def compare(val1, val2) -> int: + # IEEE 754: NaN is unordered with everything (including itself), so the + # "0 if neither > nor <" fallthrough below would wrongly report equal. + # Raise instead — callers' existing exception handling gives the right + # truth value: $eq=False, $ne=True, $lt/$lte/$gt/$gte=False. + if isinstance(val1, float) and math.isnan(val1): + raise ValueError("NaN") + if isinstance(val2, float) and math.isnan(val2): + raise ValueError("NaN") + if _is_numeric(val1) and not _is_numeric(val2): if (val2 is None): val2 = 0 @@ -124,22 +134,37 @@ def compare(val1, val2) -> int: return -1 return 0 +def _js_strict_equal(a, b) -> bool: + """JS === semantics for $eq/$ne. Routed here instead of compare() so + $lt/$gt keep their JS-aligned coercion via compare() while $eq stays + strict. + + Three buckets: + + * **Different types** → False (e.g. number 5 vs string "5", + number 1 vs boolean true). + * **Container types (array, object)** → False unconditionally. + JS `===` is reference equality for arrays/objects, and within + feature evaluation the operator's two operands always come from + separate JSON parses — different references, never `===`. So in + the only context this code observes, container $eq must be False. + * **Primitive same type** (number, string, boolean, null) → + Python `a == b`. Matches `===` for ints/floats and strings; + NaN handled correctly because `NaN == NaN` is False in Python. + """ + ta = getType(a) + if ta != getType(b): + return False + if ta == "array" or ta == "object": + return False + return bool(a == b) + + def evalOperatorCondition(operator, attributeValue, conditionValue, savedGroups) -> bool: if operator == "$eq": - try: - return compare(attributeValue, conditionValue) == 0 - except Exception: - return False + return _js_strict_equal(attributeValue, conditionValue) elif operator == "$ne": - try: - return compare(attributeValue, conditionValue) != 0 - except Exception: - # Incomparable values (e.g. missing attribute → None vs a string — - # compare() raises TypeError on the None > str comparison) are by - # definition NOT equal. Matches Mongo/JS/Go/Rust SDKs: $ne on a - # missing attribute returns True. The False default that's correct - # for $eq is inverted for $ne. - return True + return not _js_strict_equal(attributeValue, conditionValue) elif operator == "$lt": try: return compare(attributeValue, conditionValue) < 0 @@ -285,7 +310,11 @@ def _paddedVersionString(input: str) -> str: def isIn(conditionValue, attributeValue, insensitive: bool = False) -> bool: if insensitive: - # Helper function to case-fold values (lowercase for strings) + # Helper function to case-fold values (lowercase for strings). + # Uses Python str.lower(), which is byte-identical to JS toLowerCase() + # for the relevant inputs: both do Unicode-aware single-char mapping + # without multi-char folds (e.g., "İ".lower() == "i̇" in both; + # "ß".lower() == "ß" in both; "Σ".lower() == "σ" in both). def case_fold(val): return val.lower() if isinstance(val, str) else val diff --git a/tests/cases.json b/tests/cases.json index c16e641..b85b911 100644 --- a/tests/cases.json +++ b/tests/cases.json @@ -3118,6 +3118,627 @@ { "id": "3" }, false, { "group_id": [1, "2", 3] } + ], + [ + "$in - fail (case sensitive mismatch)", + { + "country": { + "$in": [ + "us", + "uk" + ] + } + }, + { + "country": "US" + }, + false + ], + [ + "$nin - pass (case sensitive mismatch)", + { + "country": { + "$nin": [ + "us", + "uk" + ] + } + }, + { + "country": "US" + }, + true + ], + [ + "$ini - pass (case insensitive match)", + { + "country": { + "$ini": [ + "us", + "uk" + ] + } + }, + { + "country": "US" + }, + true + ], + [ + "$ini - pass (uppercase pattern, lowercase value)", + { + "country": { + "$ini": [ + "US", + "UK" + ] + } + }, + { + "country": "us" + }, + true + ], + [ + "$ini - pass (mixed case)", + { + "country": { + "$ini": [ + "Us", + "Uk" + ] + } + }, + { + "country": "US" + }, + true + ], + [ + "$ini - fail (no match)", + { + "country": { + "$ini": [ + "us", + "uk" + ] + } + }, + { + "country": "CA" + }, + false + ], + [ + "$ini - array pass 1", + { + "tags": { + "$ini": [ + "a", + "b" + ] + } + }, + { + "tags": [ + "d", + "e", + "A" + ] + }, + true + ], + [ + "$ini - array pass 2", + { + "tags": { + "$ini": [ + "A", + "B" + ] + } + }, + { + "tags": [ + "d", + "b", + "f" + ] + }, + true + ], + [ + "$ini - array fail", + { + "tags": { + "$ini": [ + "a", + "b" + ] + } + }, + { + "tags": [ + "d", + "e", + "f" + ] + }, + false + ], + [ + "$ini - not array", + { + "num": { + "$ini": 1 + } + }, + { + "num": 1 + }, + false + ], + [ + "$nini - pass (case insensitive match)", + { + "country": { + "$nini": [ + "us", + "uk" + ] + } + }, + { + "country": "CA" + }, + true + ], + [ + "$nini - fail (case insensitive match)", + { + "country": { + "$nini": [ + "us", + "uk" + ] + } + }, + { + "country": "US" + }, + false + ], + [ + "$nini - fail (uppercase pattern, lowercase value)", + { + "country": { + "$nini": [ + "US", + "UK" + ] + } + }, + { + "country": "us" + }, + false + ], + [ + "$nini - array pass", + { + "tags": { + "$nini": [ + "a", + "b" + ] + } + }, + { + "tags": [ + "d", + "e", + "f" + ] + }, + true + ], + [ + "$nini - array fail 1", + { + "tags": { + "$nini": [ + "a", + "b" + ] + } + }, + { + "tags": [ + "d", + "e", + "A" + ] + }, + false + ], + [ + "$nini - array fail 2", + { + "tags": { + "$nini": [ + "A", + "B" + ] + } + }, + { + "tags": [ + "d", + "b", + "f" + ] + }, + false + ], + [ + "$nini - not array", + { + "num": { + "$nini": 1 + } + }, + { + "num": 1 + }, + false + ], + [ + "$regex - fail (case sensitive mismatch)", + { + "userAgent": { + "$regex": "(mobile|tablet)" + } + }, + { + "userAgent": "Android Mobile Browser" + }, + false + ], + [ + "$all - fail (case sensitive mismatch)", + { + "tags": { + "$all": [ + "one", + "three" + ] + } + }, + { + "tags": [ + "ONE", + "two", + "THREE" + ] + }, + false + ], + [ + "$alli - pass (case insensitive match)", + { + "tags": { + "$alli": [ + "one", + "three" + ] + } + }, + { + "tags": [ + "ONE", + "two", + "THREE" + ] + }, + true + ], + [ + "$alli - pass (uppercase pattern, lowercase value)", + { + "tags": { + "$alli": [ + "ONE", + "THREE" + ] + } + }, + { + "tags": [ + "one", + "two", + "three" + ] + }, + true + ], + [ + "$alli - pass (mixed case)", + { + "tags": { + "$alli": [ + "One", + "Three" + ] + } + }, + { + "tags": [ + "ONE", + "two", + "three" + ] + }, + true + ], + [ + "$alli - fail (case insensitive, missing value)", + { + "tags": { + "$alli": [ + "one", + "three" + ] + } + }, + { + "tags": [ + "ONE", + "two", + "four" + ] + }, + false + ], + [ + "$alli - fail not array", + { + "tags": { + "$alli": [ + "one", + "three" + ] + } + }, + { + "tags": "hello" + }, + false + ], + [ + "[D1] $lt against missing attribute coerces missing to 0 (JS spec)", + { + "x": { + "$lt": 5 + } + }, + {}, + true + ], + [ + "[D2a] $gte against missing attribute returns false (JS spec)", + { + "x": { + "$gte": 5 + } + }, + {}, + false + ], + [ + "[D2b] $lte against missing attribute returns true (JS spec)", + { + "x": { + "$lte": 5 + } + }, + {}, + true + ], + [ + "[D3] $gt against missing attribute returns false (JS spec)", + { + "x": { + "$gt": 5 + } + }, + {}, + false + ], + [ + "[D5] $elemMatch tests all array elements including falsy", + { + "x": { + "$elemMatch": { + "$eq": 0 + } + } + }, + { + "x": [ + 0 + ] + }, + true + ], + [ + "[D5] $elemMatch tests falsy false", + { + "x": { + "$elemMatch": { + "$eq": false + } + } + }, + { + "x": [ + false + ] + }, + true + ], + [ + "[D5] $elemMatch tests falsy empty string", + { + "x": { + "$elemMatch": { + "$eq": "" + } + } + }, + { + "x": [ + "" + ] + }, + true + ], + [ + "[D6a] $eq integer vs string is strict (no coercion)", + { + "x": { + "$eq": 5 + } + }, + { + "x": "5" + }, + false + ], + [ + "[D6b] equality (no operator) integer vs string is strict", + { + "x": 5 + }, + { + "x": "5" + }, + false + ], + [ + "[D6] $eq boolean vs integer is strict (different types)", + { + "x": { + "$eq": true + } + }, + { + "x": 1 + }, + false + ], + [ + "[D7] $type on empty object returns true for type 'object'", + { + "x": { + "$type": "object" + } + }, + { + "x": {} + }, + true + ], + [ + "[D10] $ini Turkish dotted-I vs ASCII i \u2014 toLowerCase produces 'i\u0307' (combining mark), not 'i'", + { + "x": { + "$ini": [ + "\u0130" + ] + } + }, + { + "x": "i" + }, + false + ], + [ + "[D10] $ini German \u00df vs SS \u2014 toLowerCase does NOT fold \u00df to ss", + { + "x": { + "$ini": [ + "STRA\u00dfE" + ] + } + }, + { + "x": "STRASSE" + }, + false + ], + [ + "[D10] $ini Greek capital sigma vs small sigma \u2014 simple Unicode fold applies", + { + "x": { + "$ini": [ + "\u03a3" + ] + } + }, + { + "x": "\u03c3" + }, + true + ], + [ + "[D10] $ini Cyrillic capital \u0410 vs small \u0430 \u2014 simple Unicode fold applies", + { + "x": { + "$ini": [ + "\u0410" + ] + } + }, + { + "x": "\u0430" + }, + true + ], + [ + "[D6c] $eq array literal vs equal array literal is strict-identity false (JS === for containers)", + { + "x": { + "$eq": [1, 2, 3] + } + }, + { + "x": [1, 2, 3] + }, + false + ], + [ + "[D6c] $eq object literal vs equal object literal is strict-identity false (JS === for containers)", + { + "x": { + "$eq": {"a": 1, "b": 2} + } + }, + { + "x": {"a": 1, "b": 2} + }, + false + ], + [ + "[D6c] $ne array literal vs equal array literal is true (negation of strict-identity)", + { + "x": { + "$ne": [1, 2, 3] + } + }, + { + "x": [1, 2, 3] + }, + true ] ], "hash": [ @@ -4898,6 +5519,57 @@ "ruleId": "" } ], + [ + "Multiple rules with divergent conditions on same prerequisite", + { + "attributes": { + "id": "u1", + "public_user_id": "pid_eci:other" + }, + "features": { + "probeFeature": { + "defaultValue": "ui_default", + "rules": [ + { + "condition": { "public_user_id": "pid_eci:does_not_match" }, + "force": "ui_default" + } + ] + }, + "childFlag": { + "defaultValue": "hello", + "rules": [ + { + "parentConditions": [ + { + "id": "probeFeature", + "condition": { "value": "ON" } + } + ], + "force": "prereqon" + }, + { + "parentConditions": [ + { + "id": "probeFeature", + "condition": { "value": { "$ne": "ON" } } + } + ], + "force": "prereqoff" + } + ] + } + } + }, + "childFlag", + { + "value": "prereqoff", + "on": true, + "off": false, + "source": "force", + "ruleId": "" + } + ], [ "SavedGroups correctly pulled from context for force rule", { @@ -4985,6 +5657,141 @@ }, "ruleId": "" } + ], + [ + "force rules - hashVersion 2 includes user", + { + "attributes": { + "id": "user2" + }, + "features": { + "feature": { + "defaultValue": 0, + "rules": [ + { + "force": 1, + "coverage": 0.5, + "hashVersion": 2 + } + ] + } + } + }, + "feature", + { + "value": 1, + "on": true, + "off": false, + "source": "force", + "ruleId": "" + } + ], + [ + "force rules - hashVersion 2 excludes user that v1 would include", + { + "attributes": { + "id": "user3" + }, + "features": { + "feature": { + "defaultValue": 0, + "rules": [ + { + "force": 1, + "coverage": 0.5, + "hashVersion": 2 + } + ] + } + } + }, + "feature", + { + "value": 0, + "on": false, + "off": true, + "source": "defaultValue", + "ruleId": "" + } + ], + [ + "[D15] parentCondition gate=false fail skips rule, continues to next", + { + "attributes": { + "id": "u1" + }, + "features": { + "B": { + "defaultValue": "off" + }, + "A": { + "defaultValue": "hello", + "rules": [ + { + "parentConditions": [ + { + "id": "B", + "condition": { + "value": "on" + }, + "gate": false + } + ], + "force": "rule1_value" + }, + { + "force": "fallback_rule" + } + ] + } + } + }, + "A", + { + "value": "fallback_rule", + "on": true, + "off": false, + "source": "force", + "ruleId": "" + } + ], + [ + "[D14] Rule combining parentConditions + force \u2014 both must pass (gate=true)", + { + "attributes": { + "id": "u1" + }, + "features": { + "B": { + "defaultValue": "on" + }, + "A": { + "defaultValue": "hello", + "rules": [ + { + "parentConditions": [ + { + "id": "B", + "condition": { + "value": "on" + }, + "gate": true + } + ], + "force": "combined_rule" + } + ] + } + } + }, + "A", + { + "value": "combined_rule", + "on": true, + "off": false, + "source": "force", + "ruleId": "" + } ] ], "run": [ @@ -6345,19 +7152,268 @@ "variationId": 1 }, { - "deviceId||d123": { - "assignments": { "feature-exp__0": "1" }, - "attributeName": "deviceId", - "attributeValue": "d123" + "deviceId||d123": { + "assignments": { "feature-exp__0": "1" }, + "attributeName": "deviceId", + "attributeValue": "d123" + } + } + ], + [ + "upgrades a sticky bucket doc from a fallbackAttribute to a hashAttribute", + { + "attributes": { + "id": "i123", + "anonymousId": "ses123", + "foo": "bar", + "country": "USA" + }, + "features": { + "exp1": { + "defaultValue": "control", + "rules": [ + { + "key": "feature-exp", + "seed": "feature-exp", + "hashAttribute": "id", + "fallbackAttribute": "anonymousId", + "hashVersion": 2, + "bucketVersion": 0, + "condition": { "country": "USA" }, + "variations": ["control", "red", "blue"], + "meta": [{ "key": "0" }, { "key": "1" }, { "key": "2" }], + "coverage": 1, + "weights": [0.3334, 0.3333, 0.3333], + "phase": "0" + } + ] + } + } + }, + [ + { + "attributeName": "anonymousId", + "attributeValue": "ses123", + "assignments": { + "feature-exp__0": "1" + } + } + ], + "exp1", + { + "bucket": 0.9943, + "featureId": "exp1", + "hashAttribute": "id", + "hashUsed": true, + "hashValue": "i123", + "inExperiment": true, + "key": "1", + "stickyBucketUsed": true, + "value": "red", + "variationId": 1 + }, + { + "anonymousId||ses123": { + "assignments": { "feature-exp__0": "1" }, + "attributeName": "anonymousId", + "attributeValue": "ses123" + }, + "id||i123": { + "assignments": { "feature-exp__0": "1" }, + "attributeName": "id", + "attributeValue": "i123" + } + } + ], + [ + "favors a sticky bucket doc based on hashAttribute over fallbackAttribute", + { + "attributes": { + "id": "i123", + "anonymousId": "ses123", + "foo": "bar", + "country": "USA" + }, + "features": { + "exp1": { + "defaultValue": "control", + "rules": [ + { + "key": "feature-exp", + "seed": "feature-exp", + "hashAttribute": "id", + "fallbackAttribute": "anonymousId", + "hashVersion": 2, + "bucketVersion": 0, + "condition": { "country": "USA" }, + "variations": ["control", "red", "blue"], + "meta": [{ "key": "0" }, { "key": "1" }, { "key": "2" }], + "coverage": 1, + "weights": [0.3334, 0.3333, 0.3333], + "phase": "0" + } + ] + } + } + }, + [ + { + "attributeName": "anonymousId", + "attributeValue": "ses123", + "assignments": { + "feature-exp__0": "2" + } + }, + { + "attributeName": "id", + "attributeValue": "i123", + "assignments": { + "feature-exp__0": "1" + } + } + ], + "exp1", + { + "bucket": 0.9943, + "featureId": "exp1", + "hashAttribute": "id", + "hashUsed": true, + "hashValue": "i123", + "inExperiment": true, + "key": "1", + "stickyBucketUsed": true, + "value": "red", + "variationId": 1 + }, + { + "anonymousId||ses123": { + "assignments": { "feature-exp__0": "2" }, + "attributeName": "anonymousId", + "attributeValue": "ses123" + }, + "id||i123": { + "assignments": { "feature-exp__0": "1" }, + "attributeName": "id", + "attributeValue": "i123" + } + } + ], + [ + "resets sticky bucketing when the bucketVersion changes", + { + "attributes": { + "id": "i123", + "foo": "bar", + "country": "USA" + }, + "features": { + "exp1": { + "defaultValue": "control", + "rules": [ + { + "key": "feature-exp", + "seed": "feature-exp", + "hashAttribute": "id", + "fallbackAttribute": "deviceId", + "hashVersion": 2, + "bucketVersion": 3, + "condition": { "country": "USA" }, + "variations": ["control", "red", "blue"], + "meta": [{ "key": "0" }, { "key": "1" }, { "key": "2" }], + "coverage": 1, + "weights": [0.3334, 0.3333, 0.3333], + "phase": "0" + } + ] + } + } + }, + [ + { + "assignments": { "feature-exp__0": "1" }, + "attributeName": "id", + "attributeValue": "i123" + } + ], + "exp1", + { + "bucket": 0.9943, + "featureId": "exp1", + "hashAttribute": "id", + "hashUsed": true, + "hashValue": "i123", + "inExperiment": true, + "key": "2", + "stickyBucketUsed": false, + "value": "blue", + "variationId": 2 + }, + { + "id||i123": { + "assignments": { + "feature-exp__0": "1", + "feature-exp__3": "2" + }, + "attributeName": "id", + "attributeValue": "i123" + } + } + ], + [ + "stops test enrollment when and existing sticky bucket is blocked by version", + { + "attributes": { + "id": "i123", + "foo": "bar", + "country": "USA" + }, + "features": { + "exp1": { + "defaultValue": "control", + "rules": [ + { + "key": "feature-exp", + "seed": "feature-exp", + "hashAttribute": "id", + "fallbackAttribute": "deviceId", + "hashVersion": 2, + "bucketVersion": 3, + "minBucketVersion": 3, + "condition": { "country": "USA" }, + "variations": ["control", "red", "blue"], + "meta": [{ "key": "0" }, { "key": "1" }, { "key": "2" }], + "coverage": 1, + "weights": [0.3334, 0.3333, 0.3333], + "phase": "0" + } + ] + } + } + }, + [ + { + "assignments": { "feature-exp__0": "1" }, + "attributeName": "id", + "attributeValue": "i123" + } + ], + "exp1", + null, + { + "id||i123": { + "assignments": { + "feature-exp__0": "1" + }, + "attributeName": "id", + "attributeValue": "i123" } } ], [ - "upgrades a sticky bucket doc from a fallbackAttribute to a hashAttribute", + "disables sticky bucketing when disabled by experiment", { "attributes": { "id": "i123", - "anonymousId": "ses123", "foo": "bar", "country": "USA" }, @@ -6369,9 +7425,10 @@ "key": "feature-exp", "seed": "feature-exp", "hashAttribute": "id", - "fallbackAttribute": "anonymousId", + "fallbackAttribute": "deviceId", "hashVersion": 2, - "bucketVersion": 0, + "bucketVersion": 1, + "disableStickyBucketing": true, "condition": { "country": "USA" }, "variations": ["control", "red", "blue"], "meta": [{ "key": "0" }, { "key": "1" }, { "key": "2" }], @@ -6385,11 +7442,9 @@ }, [ { - "attributeName": "anonymousId", - "attributeValue": "ses123", - "assignments": { - "feature-exp__0": "1" - } + "attributeName": "id", + "attributeValue": "i123", + "assignments": { "feature-exp__0": "1" } } ], "exp1", @@ -6400,29 +7455,24 @@ "hashUsed": true, "hashValue": "i123", "inExperiment": true, - "key": "1", - "stickyBucketUsed": true, - "value": "red", - "variationId": 1 + "key": "2", + "stickyBucketUsed": false, + "value": "blue", + "variationId": 2 }, { - "anonymousId||ses123": { - "assignments": { "feature-exp__0": "1" }, - "attributeName": "anonymousId", - "attributeValue": "ses123" - }, "id||i123": { - "assignments": { "feature-exp__0": "1" }, "attributeName": "id", - "attributeValue": "i123" + "attributeValue": "i123", + "assignments": { "feature-exp__0": "1" } } } ], [ - "favors a sticky bucket doc based on hashAttribute over fallbackAttribute", + "uses a sticky bucket when sticky bucket version == experiment.minBucketVersion === experiment.bucketVersion", { "attributes": { - "id": "i123", + "deviceId": "d123", "anonymousId": "ses123", "foo": "bar", "country": "USA" @@ -6435,14 +7485,35 @@ "key": "feature-exp", "seed": "feature-exp", "hashAttribute": "id", - "fallbackAttribute": "anonymousId", + "fallbackAttribute": "deviceId", "hashVersion": 2, - "bucketVersion": 0, - "condition": { "country": "USA" }, - "variations": ["control", "red", "blue"], - "meta": [{ "key": "0" }, { "key": "1" }, { "key": "2" }], + "bucketVersion": 3, + "minBucketVersion": 3, + "condition": { + "country": "USA" + }, + "variations": [ + "control", + "red", + "blue" + ], + "meta": [ + { + "key": "0" + }, + { + "key": "1" + }, + { + "key": "2" + } + ], "coverage": 1, - "weights": [0.3334, 0.3333, 0.3333], + "weights": [ + 0.3334, + 0.3333, + 0.3333 + ], "phase": "0" } ] @@ -6451,51 +7522,42 @@ }, [ { - "attributeName": "anonymousId", - "attributeValue": "ses123", - "assignments": { - "feature-exp__0": "2" - } - }, - { - "attributeName": "id", - "attributeValue": "i123", + "attributeName": "deviceId", + "attributeValue": "d123", "assignments": { - "feature-exp__0": "1" + "feature-exp__3": "2" } } ], "exp1", { - "bucket": 0.9943, + "bucket": 0.6468, "featureId": "exp1", - "hashAttribute": "id", + "hashAttribute": "deviceId", "hashUsed": true, - "hashValue": "i123", + "hashValue": "d123", "inExperiment": true, - "key": "1", + "key": "2", "stickyBucketUsed": true, - "value": "red", - "variationId": 1 + "value": "blue", + "variationId": 2 }, { - "anonymousId||ses123": { - "assignments": { "feature-exp__0": "2" }, - "attributeName": "anonymousId", - "attributeValue": "ses123" - }, - "id||i123": { - "assignments": { "feature-exp__0": "1" }, - "attributeName": "id", - "attributeValue": "i123" + "deviceId||d123": { + "assignments": { + "feature-exp__3": "2" + }, + "attributeName": "deviceId", + "attributeValue": "d123" } } ], [ - "resets sticky bucketing when the bucketVersion changes", + "skips assignment when sticky bucket version < experiment.minBucketVersion", { "attributes": { - "id": "i123", + "deviceId": "d123", + "anonymousId": "ses123", "foo": "bar", "country": "USA" }, @@ -6510,11 +7572,32 @@ "fallbackAttribute": "deviceId", "hashVersion": 2, "bucketVersion": 3, - "condition": { "country": "USA" }, - "variations": ["control", "red", "blue"], - "meta": [{ "key": "0" }, { "key": "1" }, { "key": "2" }], + "minBucketVersion": 3, + "condition": { + "country": "USA" + }, + "variations": [ + "control", + "red", + "blue" + ], + "meta": [ + { + "key": "0" + }, + { + "key": "1" + }, + { + "key": "2" + } + ], "coverage": 1, - "weights": [0.3334, 0.3333, 0.3333], + "weights": [ + 0.3334, + 0.3333, + 0.3333 + ], "phase": "0" } ] @@ -6523,40 +7606,31 @@ }, [ { - "assignments": { "feature-exp__0": "1" }, - "attributeName": "id", - "attributeValue": "i123" + "attributeName": "deviceId", + "attributeValue": "d123", + "assignments": { + "feature-exp__2": "2" + } } ], "exp1", + null, { - "bucket": 0.9943, - "featureId": "exp1", - "hashAttribute": "id", - "hashUsed": true, - "hashValue": "i123", - "inExperiment": true, - "key": "2", - "stickyBucketUsed": false, - "value": "blue", - "variationId": 2 - }, - { - "id||i123": { + "deviceId||d123": { "assignments": { - "feature-exp__0": "1", - "feature-exp__3": "2" + "feature-exp__2": "2" }, - "attributeName": "id", - "attributeValue": "i123" + "attributeName": "deviceId", + "attributeValue": "d123" } } ], [ - "stops test enrollment when and existing sticky bucket is blocked by version", + "resets sticky bucketing when bucket version > experiment.bucketVersion (invalid version)", { "attributes": { - "id": "i123", + "deviceId": "d123", + "anonymousId": "ses123", "foo": "bar", "country": "USA" }, @@ -6572,11 +7646,31 @@ "hashVersion": 2, "bucketVersion": 3, "minBucketVersion": 3, - "condition": { "country": "USA" }, - "variations": ["control", "red", "blue"], - "meta": [{ "key": "0" }, { "key": "1" }, { "key": "2" }], + "condition": { + "country": "USA" + }, + "variations": [ + "control", + "red", + "blue" + ], + "meta": [ + { + "key": "0" + }, + { + "key": "1" + }, + { + "key": "2" + } + ], "coverage": 1, - "weights": [0.3334, 0.3333, 0.3333], + "weights": [ + 0.3334, + 0.3333, + 0.3333 + ], "phase": "0" } ] @@ -6585,28 +7679,43 @@ }, [ { - "assignments": { "feature-exp__0": "1" }, - "attributeName": "id", - "attributeValue": "i123" + "attributeName": "deviceId", + "attributeValue": "d123", + "assignments": { + "feature-exp__4": "2" + } } ], "exp1", - null, { - "id||i123": { + "bucket": 0.6468, + "featureId": "exp1", + "hashAttribute": "deviceId", + "hashUsed": true, + "hashValue": "d123", + "inExperiment": true, + "key": "1", + "stickyBucketUsed": false, + "value": "red", + "variationId": 1 + }, + { + "deviceId||d123": { "assignments": { - "feature-exp__0": "1" + "feature-exp__3": "1", + "feature-exp__4": "2" }, - "attributeName": "id", - "attributeValue": "i123" + "attributeName": "deviceId", + "attributeValue": "d123" } } ], [ - "disables sticky bucketing when disabled by experiment", + "resets sticky bucketing when bucket version < experiment.bucketVersion (invalid version)", { "attributes": { - "id": "i123", + "deviceId": "d123", + "anonymousId": "ses123", "foo": "bar", "country": "USA" }, @@ -6620,13 +7729,33 @@ "hashAttribute": "id", "fallbackAttribute": "deviceId", "hashVersion": 2, - "bucketVersion": 1, - "disableStickyBucketing": true, - "condition": { "country": "USA" }, - "variations": ["control", "red", "blue"], - "meta": [{ "key": "0" }, { "key": "1" }, { "key": "2" }], + "bucketVersion": 4, + "minBucketVersion": 3, + "condition": { + "country": "USA" + }, + "variations": [ + "control", + "red", + "blue" + ], + "meta": [ + { + "key": "0" + }, + { + "key": "1" + }, + { + "key": "2" + } + ], "coverage": 1, - "weights": [0.3334, 0.3333, 0.3333], + "weights": [ + 0.3334, + 0.3333, + 0.3333 + ], "phase": "0" } ] @@ -6635,29 +7764,34 @@ }, [ { - "attributeName": "id", - "attributeValue": "i123", - "assignments": { "feature-exp__0": "1" } + "attributeName": "deviceId", + "attributeValue": "d123", + "assignments": { + "feature-exp__3": "2" + } } ], "exp1", { - "bucket": 0.9943, + "bucket": 0.6468, "featureId": "exp1", - "hashAttribute": "id", + "hashAttribute": "deviceId", "hashUsed": true, - "hashValue": "i123", + "hashValue": "d123", "inExperiment": true, - "key": "2", + "key": "1", "stickyBucketUsed": false, - "value": "blue", - "variationId": 2 + "value": "red", + "variationId": 1 }, { - "id||i123": { - "attributeName": "id", - "attributeValue": "i123", - "assignments": { "feature-exp__0": "1" } + "deviceId||d123": { + "assignments": { + "feature-exp__3": "2", + "feature-exp__4": "1" + }, + "attributeName": "deviceId", + "attributeValue": "d123" } } ] diff --git a/tests/scripts/check_corpus_freshness.py b/tests/scripts/check_corpus_freshness.py new file mode 100644 index 0000000..0661fd0 --- /dev/null +++ b/tests/scripts/check_corpus_freshness.py @@ -0,0 +1,358 @@ +#!/usr/bin/env python3 +"""Check Python's tests/cases.json against the JS SDK's cases.json. + +The two corpora are maintained by hand; `specVersion` is a label, not a +contract. This script diffs the corpora and makes drift visible. + +Diff categories: + + - "missing" — JS has a case name Python doesn't. + Fails CI unless the name is in skiplist["missing"][key]. + - "drift" — Both sides have the case name, but the bodies differ + (canonical-JSON SHA1 mismatch). Fails CI unless the name + is in skiplist["drift"][key]. Catches the silent + case-body update that pure name-matching misses. + - "extra" — Python has a case name JS doesn't. Reported as + informational only — Python carries documented + extensions plus locally-authored regressions. Never + fails CI. + +Source-of-truth URL is configurable via --js-source or env GB_JS_CASES_URL. +Defaults to the JS SDK's main-branch raw URL. + +Exit codes: + 0 — no actionable findings (or all on skiplist) + 1 — at least one missing or drifted case isn't on the skiplist + 2 — fetch / parse / IO error (treated as build infra failure) + +Run locally: + python3 tests/scripts/check_corpus_freshness.py + python3 tests/scripts/check_corpus_freshness.py --js-source /path/to/local/cases.json + GB_JS_CASES_URL=https://... python3 tests/scripts/check_corpus_freshness.py + +In CI, this runs on every push as a separate step in the build workflow. +""" + +from __future__ import annotations + +import argparse +import hashlib +import json +import os +import sys +import urllib.error +import urllib.request +from collections import Counter +from pathlib import Path +from typing import Dict, List, Set, Tuple + +REPO_ROOT = Path(__file__).resolve().parents[2] +LOCAL_CASES = REPO_ROOT / "tests" / "cases.json" +SKIPLIST = REPO_ROOT / "tests" / "scripts" / "corpus_skiplist.json" + +DEFAULT_JS_URL = ( + "https://raw.githubusercontent.com/growthbook/growthbook/main/" + "packages/sdk-js/test/cases.json" +) + +# Top-level keys to diff. Other keys in cases.json (specVersion, decrypt +# binary blobs, urlRedirect which Python doesn't yet wire) are skipped +# either because they're scalar metadata or because the divergence is +# tracked separately. +KEYS_TO_DIFF = ( + "evalCondition", + "feature", + "run", + "hash", + "getBucketRange", + "chooseVariation", + "getQueryStringOverride", + "inNamespace", + "getEqualWeights", + "stickyBucket", +) + + +def _fetch_js_cases(source: str) -> dict: + """Fetch JS cases.json from a URL or local path. + + Raises RuntimeError with a human-readable message on failure. + """ + if source.startswith(("http://", "https://")): + try: + req = urllib.request.Request( + source, headers={"User-Agent": "growthbook-python-corpus-check"} + ) + with urllib.request.urlopen(req, timeout=20) as resp: # noqa: S310 + return json.loads(resp.read().decode("utf-8")) + except urllib.error.URLError as e: + raise RuntimeError(f"fetch failed: {source}: {e}") from e + except json.JSONDecodeError as e: + raise RuntimeError(f"JS source did not return valid JSON: {e}") from e + path = Path(source) + if not path.is_file(): + raise RuntimeError(f"local source not found: {source}") + try: + return json.loads(path.read_text(encoding="utf-8")) + except json.JSONDecodeError as e: + raise RuntimeError(f"local source invalid JSON: {e}") from e + + +def _load_local_cases() -> dict: + if not LOCAL_CASES.is_file(): + raise RuntimeError(f"local cases.json not found: {LOCAL_CASES}") + return json.loads(LOCAL_CASES.read_text(encoding="utf-8")) + + +def _load_skiplist() -> Dict[str, Dict[str, Set[str]]]: + """Load skiplist. File format: + + { + "missing": { "": ["case name", ...] }, + "drift": { "": ["case name", ...] } + } + + `missing` — case names Python deliberately doesn't carry from JS. + `drift` — case names where Python deliberately diverges from JS's + body (rare; reserved for cases that test a Python-only + extension behavior). + + Extras (Python has, JS doesn't) are reported but never fail, so they + don't need a skiplist entry. The file is optional. + """ + if not SKIPLIST.is_file(): + return {"missing": {}, "drift": {}} + try: + data = json.loads(SKIPLIST.read_text(encoding="utf-8")) + except json.JSONDecodeError as e: + raise RuntimeError(f"skiplist invalid JSON: {e}") from e + return { + "missing": {k: set(v) for k, v in (data.get("missing") or {}).items()}, + "drift": {k: set(v) for k, v in (data.get("drift") or {}).items()}, + } + + +def _case_signatures_grouped(cases: list) -> Dict[str, List[str]]: + """Return {case_name: [body_hash, body_hash, ...]} preserving order. + + Body = everything after the name (case[1:]), serialized via canonical + JSON (sorted keys + compact separators) so logically-equal cases hash + the same regardless of dict key insertion order or whitespace. + + Same-named cases keep every occurrence in the list because pytest's + `pytest_generate_tests` parametrizes the FULL case list — so all + duplicates run. Collapsing them to a single entry would silently hide + body drift in any occurrence except the last (the original bug here). + + Drift detection compares per-name multisets, so the order of + duplicates within a name doesn't matter; only the set of body hashes + does. + """ + out: Dict[str, List[str]] = {} + for c in cases: + if not (isinstance(c, list) and c and isinstance(c[0], str)): + continue + name = c[0] + body = json.dumps(c[1:], sort_keys=True, separators=(",", ":")) + h = hashlib.sha1(body.encode("utf-8")).hexdigest()[:16] + out.setdefault(name, []).append(h) + return out + + +def _diff( + js_cases: dict, py_cases: dict, skip: Dict[str, Set[str]] +) -> Tuple[ + Dict[str, List[str]], # actionable_missing + Dict[str, List[str]], # skipped_missing + Dict[str, List[str]], # extras + Dict[str, List[str]], # actionable_drift (same name, different body) + Dict[str, List[str]], # skipped_drift +]: + """Diff (name, body-hash) pairs. + + Returns five per-key dictionaries: + * actionable_missing — JS has the name, Python doesn't (fails CI) + * skipped_missing — same, but the name is on the skiplist + * extras — Python has the name, JS doesn't (never fails) + * actionable_drift — same name, hash differs (fails CI) + * skipped_drift — same name, hash differs, name on drift-skiplist + """ + actionable_missing: Dict[str, List[str]] = {} + skipped_missing: Dict[str, List[str]] = {} + extras: Dict[str, List[str]] = {} + actionable_drift: Dict[str, List[str]] = {} + skipped_drift: Dict[str, List[str]] = {} + + missing_skip = skip.get("missing", {}) + drift_skip = skip.get("drift", {}) + + for key in KEYS_TO_DIFF: + js_list = js_cases.get(key, []) + py_list = py_cases.get(key, []) + if not isinstance(js_list, list) or not isinstance(py_list, list): + continue + + js_grouped = _case_signatures_grouped(js_list) + py_grouped = _case_signatures_grouped(py_list) + + # Preserve JS file ordering for the missing/drift reports; iterate + # the original list and dedupe so each unique name appears once. + js_names_ordered: List[str] = [] + seen_js: Set[str] = set() + for c in js_list: + if isinstance(c, list) and c and isinstance(c[0], str) and c[0] not in seen_js: + js_names_ordered.append(c[0]) + seen_js.add(c[0]) + + py_names_ordered: List[str] = [] + seen_py: Set[str] = set() + for c in py_list: + if isinstance(c, list) and c and isinstance(c[0], str) and c[0] not in seen_py: + py_names_ordered.append(c[0]) + seen_py.add(c[0]) + + missing = [n for n in js_names_ordered if n not in py_grouped] + extra = [n for n in py_names_ordered if n not in js_grouped] + + # Drift = same name on both sides, but the multiset of body hashes + # differs. Catches single-occurrence drift AND drift where only + # ONE of several duplicates changed (the original bug). + drift = [ + n for n in js_names_ordered + if n in py_grouped and Counter(js_grouped[n]) != Counter(py_grouped[n]) + ] + + key_missing_skip = missing_skip.get(key, set()) + key_drift_skip = drift_skip.get(key, set()) + + actionable_missing[key] = [n for n in missing if n not in key_missing_skip] + skipped_missing[key] = [n for n in missing if n in key_missing_skip] + extras[key] = extra + actionable_drift[key] = [n for n in drift if n not in key_drift_skip] + skipped_drift[key] = [n for n in drift if n in key_drift_skip] + + return actionable_missing, skipped_missing, extras, actionable_drift, skipped_drift + + +def _spec_versions(js_cases: dict, py_cases: dict) -> Tuple[str, str]: + return ( + str(js_cases.get("specVersion", "")), + str(py_cases.get("specVersion", "")), + ) + + +def _format_report( + js_spec: str, + py_spec: str, + actionable_missing: Dict[str, List[str]], + skipped_missing: Dict[str, List[str]], + extras: Dict[str, List[str]], + actionable_drift: Dict[str, List[str]], + skipped_drift: Dict[str, List[str]], +) -> str: + lines = [] + lines.append("=== Corpus freshness check (Python vs JS SDK) ===") + lines.append(f" JS specVersion: {js_spec}") + lines.append(f" Python specVersion: {py_spec}") + if js_spec != py_spec: + lines.append( + " ⚠ specVersion mismatch — bump Python's value when you " + "catch up to JS's." + ) + lines.append("") + + n_missing = sum(len(v) for v in actionable_missing.values()) + n_skip_missing = sum(len(v) for v in skipped_missing.values()) + n_drift = sum(len(v) for v in actionable_drift.values()) + n_skip_drift = sum(len(v) for v in skipped_drift.values()) + n_extra = sum(len(v) for v in extras.values()) + + n_fail = n_missing + n_drift + if n_fail == 0: + lines.append( + f"OK: no missing/drifted cases " + f"(skipped-missing: {n_skip_missing}, skipped-drift: {n_skip_drift}, extras: {n_extra})" + ) + else: + lines.append( + f"DRIFT: {n_missing} missing + {n_drift} body-drift " + f"(skipped: {n_skip_missing} missing, {n_skip_drift} drift; " + f"{n_extra} Python extras)" + ) + lines.append("") + + def _section(title: str, data: Dict[str, List[str]]) -> None: + if not any(data.values()): + return + lines.append(f"--- {title} ---") + for key, names in data.items(): + if not names: + continue + lines.append(f" [{key}] ({len(names)})") + for n in names: + lines.append(f" - {n}") + lines.append("") + + _section("Missing in Python (FAILS CI)", actionable_missing) + _section("Body-drift: same name, different case body (FAILS CI)", actionable_drift) + _section("Missing in Python — skipped via corpus_skiplist.json", skipped_missing) + _section("Body-drift — skipped via corpus_skiplist.json", skipped_drift) + _section("Extra in Python (informational; never fails)", extras) + return "\n".join(lines) + + +def main(argv: List[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__.split("\n\n")[0]) + parser.add_argument( + "--js-source", + default=os.environ.get("GB_JS_CASES_URL", DEFAULT_JS_URL), + help="URL or local path to JS cases.json (default: JS SDK main branch)", + ) + parser.add_argument( + "--json", action="store_true", help="output machine-readable JSON instead of text" + ) + args = parser.parse_args(argv) + + try: + js_cases = _fetch_js_cases(args.js_source) + py_cases = _load_local_cases() + skip = _load_skiplist() + except RuntimeError as e: + print(f"corpus check infra error: {e}", file=sys.stderr) + return 2 + + actionable_missing, skipped_missing, extras, actionable_drift, skipped_drift = _diff( + js_cases, py_cases, skip + ) + js_spec, py_spec = _spec_versions(js_cases, py_cases) + + if args.json: + print( + json.dumps( + { + "js_specVersion": js_spec, + "py_specVersion": py_spec, + "missing_actionable": actionable_missing, + "missing_skipped": skipped_missing, + "drift_actionable": actionable_drift, + "drift_skipped": skipped_drift, + "extras": extras, + }, + indent=2, + sort_keys=True, + ) + ) + else: + print( + _format_report( + js_spec, py_spec, actionable_missing, skipped_missing, + extras, actionable_drift, skipped_drift, + ) + ) + + fail = any(actionable_missing.values()) or any(actionable_drift.values()) + return 1 if fail else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/scripts/corpus_skiplist.json b/tests/scripts/corpus_skiplist.json new file mode 100644 index 0000000..49a54ce --- /dev/null +++ b/tests/scripts/corpus_skiplist.json @@ -0,0 +1,9 @@ +{ + "_doc": "Skiplist for the corpus freshness check. Two buckets:\n * \"missing\" — case names JS has and Python deliberately doesn't. Use when JS adds a case for a feature Python doesn't (yet) support.\n * \"drift\" — case names where Python deliberately keeps a different body from JS. Use sparingly — body-drift on a shared case is usually a bug.\nExtras (Python has, JS doesn't) are reported but never fail CI, so they don't need an entry here.\nAdd a brief reason in `_reasons` whenever you add an entry so future maintainers know why.", + "missing": {}, + "drift": {}, + "_reasons": { + "_example_missing": "evalCondition::case name -> short reason (e.g. 'tests $someOperator that Python doesn't implement yet').", + "_example_drift": "evalCondition::case name -> short reason (e.g. 'JS shipped a behavioral change in case X; Python intentionally kept the prior expectation pending review')." + } +} diff --git a/tests/test_growthbook.py b/tests/test_growthbook.py index 809b08b..b51f6a6 100644 --- a/tests/test_growthbook.py +++ b/tests/test_growthbook.py @@ -106,6 +106,29 @@ def test_conditions(evalCondition_data): assert evalCondition(attributes, condition, savedGroups) == expected +def test_nan_attribute_obeys_ieee_754(): + # JSON can't represent NaN, so this can't live in cases.json. Without + # the explicit NaN guard in compare(), compare(NaN, NaN) returns 0 + # because both > and < are False — wrongly reporting $eq match. + # IEEE 754: every comparison involving NaN is False except !=. + nan = float("nan") + cond_nan = {"x": nan} + + assert evalCondition({"x": nan}, {"x": {"$eq": nan}}) is False + assert evalCondition({"x": nan}, {"x": {"$ne": nan}}) is True + assert evalCondition({"x": nan}, {"x": {"$lt": nan}}) is False + assert evalCondition({"x": nan}, {"x": {"$lte": nan}}) is False + assert evalCondition({"x": nan}, {"x": {"$gt": nan}}) is False + assert evalCondition({"x": nan}, {"x": {"$gte": nan}}) is False + # NaN attribute vs concrete number: every comparison is False. + assert evalCondition({"x": nan}, {"x": {"$eq": 5}}) is False + assert evalCondition({"x": nan}, {"x": {"$lt": 5}}) is False + assert evalCondition({"x": nan}, {"x": {"$gt": 5}}) is False + # NaN condition vs concrete attribute: same. + assert evalCondition({"x": 5}, {"x": {"$eq": nan}}) is False + assert evalCondition({"x": 5}, {"x": {"$ne": nan}}) is True + + def test_version_comparison_normalizes_unhashable_values(): assert paddedVersionString(["1.2.3"]) == paddedVersionString("0") assert paddedVersionString({"version": "1.2.3"}) == paddedVersionString("0")