From 10eb8f257fa136b66b83578b32b4751b1b374a20 Mon Sep 17 00:00:00 2001 From: tarsis-viana Date: Thu, 8 Oct 2026 11:59:55 +0100 Subject: [PATCH 1/7] feat(FT-2287): apply split traffic override rules An override rule can now split matched units across the experiment's variants instead of assigning one fixed variant. Matched units are hashed with the experiment's own seed, so a rule split equal to the experiment split keeps every unit on the variant it already had. A split rule otherwise behaves exactly like an assign rule: same precedence (beats holdouts, audience, traffic, full-on and custom assignments; loses only to override()) and same exposure flags. A split rule that matches while the unit is missing assigns variant 0 instead of falling through to a later rule, and is re-resolved once the unit is set. Malformed split rules are skipped like an invalid assign variant. Co-Authored-By: Claude Opus 5.5 --- src/__tests__/context.test.js | 169 ++++++++++++++++++++++++++++++++++ src/__tests__/matcher.test.js | 78 ++++++++++++---- src/context.ts | 48 +++++----- src/matcher.ts | 38 ++++++-- 4 files changed, 285 insertions(+), 48 deletions(-) diff --git a/src/__tests__/context.test.js b/src/__tests__/context.test.js index fe91509..f035f33 100644 --- a/src/__tests__/context.test.js +++ b/src/__tests__/context.test.js @@ -2900,6 +2900,105 @@ describe("Context", () => { done(); }); }); + + describe("split rules", () => { + const splitRulesResponse = (percentages, otherRules = []) => + buildRulesResponse({ + assignmentRules: JSON.stringify({ + rules: [ + { + name: "US Users", + type: "split", + conditions: { and: [{ eq: [{ var: "country" }, { value: "US" }] }] }, + environments: [], + percentages, + }, + ...otherRules, + ], + }), + }); + + it("should hash the unit with the experiment seed, so the experiment's own split keeps its variant", () => { + const context = new Context(sdk, contextOptions, contextParams, splitRulesResponse("34/33/33")); + context.attribute("country", "US"); + expect(context.treatment("exp_test_abc")).toEqual(expectedVariants["exp_test_abc"]); + }); + + it("should not hash the unit with the traffic seed", () => { + const context = new Context(sdk, contextOptions, contextParams, splitRulesResponse("0/70/30")); + context.attribute("country", "US"); + expect(context.treatment("exp_test_abc")).toEqual(2); + }); + + it("should assign the only variant with a non-zero share", () => { + const context = new Context(sdk, contextOptions, contextParams, splitRulesResponse("0/100/0")); + context.attribute("country", "US"); + expect(context.treatment("exp_test_abc")).toEqual(1); + }); + + it("should set the same exposure flags as an assign rule", (done) => { + const context = new Context(sdk, contextOptions, contextParams, splitRulesResponse("0/100/0")); + context.attribute("country", "US"); + context.treatment("exp_test_abc"); + + publisher.publish.mockReturnValue(Promise.resolve()); + + context.publish().then(() => { + const exposure = publisher.publish.mock.calls[0][0].exposures.find( + (e) => e.name === "exp_test_abc" + ); + expect(exposure).toEqual({ + id: 2, + name: "exp_test_abc", + unit: "session_id", + exposedAt: timeOrigin, + variant: 1, + assigned: false, + eligible: true, + overridden: false, + fullOn: false, + custom: false, + audienceMismatch: false, + ruleOverride: true, + }); + done(); + }); + }); + + it("should take priority over a custom assignment", () => { + const context = new Context(sdk, contextOptions, contextParams, splitRulesResponse("0/100/0")); + context.attribute("country", "US"); + context.customAssignment("exp_test_abc", 2); + expect(context.treatment("exp_test_abc")).toEqual(1); + }); + + it("should fall back to normal assignment when the split does not cover every variant", () => { + const context = new Context(sdk, contextOptions, contextParams, splitRulesResponse("0/100")); + context.attribute("country", "US"); + expect(context.treatment("exp_test_abc")).toEqual(expectedVariants["exp_test_abc"]); + }); + + it("should assign variant 0 and stop at the split rule while the unit is missing, then re-resolve once it is set", () => { + const laterAssignRule = { + name: "Everyone", + type: "assign", + conditions: null, + environments: [], + variant: 2, + }; + const context = new Context( + sdk, + contextOptions, + { units: { user_id: contextParams.units.user_id } }, + splitRulesResponse("0/100/0", [laterAssignRule]) + ); + context.attribute("country", "US"); + expect(context.peek("exp_test_abc")).toEqual(0); + + context.unit("session_id", contextParams.units.session_id); + expect(context.peek("exp_test_abc")).toEqual(1); + }); + }); }); describe("holdouts", () => { @@ -4845,6 +4944,76 @@ describe("Context", () => { }); }); + it("lets a matching split rule win over holdout suppression", () => { + const response = buildHoldoutResponse( + [ + { + id: 1, + name: "exp_holdout_split_rule", + iteration: 1, + unitType: "session_id", + seedHi: 100, + seedLo: 200, + split: [0.5, 0.5], + trafficSeedHi: 1, + trafficSeedLo: 2, + trafficSplit: [0, 1], + fullOnVariant: 0, + applications: [{ name: "website" }], + variants: [ + { name: "A", config: null }, + { name: "B", config: null }, + ], + audience: null, + audienceStrict: false, + assignmentRules: JSON.stringify({ + rules: [ + { + name: "US Users", + type: "split", + conditions: { and: [{ eq: [{ var: "country" }, { value: "US" }] }] }, + environments: [], + percentages: "0/100", + }, + ], + }), + customFieldValues: null, + holdoutIds: [11], + }, + ], + [ + { + id: 11, + name: "holdout_split_rule_suppression", + iteration: 1, + unitType: "session_id", + seedHi: 13, + seedLo: 111, + split: [0.1, 0.9], + trafficSeedHi: 0, + trafficSeedLo: 0, + trafficSplit: [0, 1], + fullOnVariant: 0, + applications: [], + variants: [ + { name: "A", config: null }, + { name: "B", config: null }, + ], + audience: null, + audienceStrict: false, + customFieldValues: null, + holdoutType: "full", + }, + ] + ); + + const context = new Context(sdk, contextOptions, contextParams, response); + context.attribute("country", "US"); + + expect(context.treatment("exp_holdout_split_rule")).toEqual(1); + expect(context._assignments["exp_holdout_split_rule"].suppressed).toEqual(true); + }); + // Final-review Finding I-2, Test A: regression coverage for Task 5's override-fast-path // fix (commit 716430c). Before that fix, the `hasOverride` branch in `_assign()` returned // the cached assignment early on `overridden && variant match` alone, without checking diff --git a/src/__tests__/matcher.test.js b/src/__tests__/matcher.test.js index ecc52d4..a6b458c 100644 --- a/src/__tests__/matcher.test.js +++ b/src/__tests__/matcher.test.js @@ -48,7 +48,7 @@ describe("AudienceMatcher", () => { }, ], }); - expect(matcher.evaluateRules(audience, "production", { country: "US" })).toBe(1); + expect(matcher.evaluateRules(audience, "production", { country: "US" })).toEqual({ variant: 1 }); }); it("should return null when conditions do not match", () => { @@ -93,8 +93,8 @@ describe("AudienceMatcher", () => { }, ], }); - expect(matcher.evaluateRules(audience, "production", { country: "US" })).toBe(2); - expect(matcher.evaluateRules(audience, "staging", { country: "US" })).toBe(2); + expect(matcher.evaluateRules(audience, "production", { country: "US" })).toEqual({ variant: 2 }); + expect(matcher.evaluateRules(audience, "staging", { country: "US" })).toEqual({ variant: 2 }); }); it("should match all environments when environments is empty", () => { @@ -109,9 +109,9 @@ describe("AudienceMatcher", () => { }, ], }); - expect(matcher.evaluateRules(audience, "production", {})).toBe(1); - expect(matcher.evaluateRules(audience, "staging", {})).toBe(1); - expect(matcher.evaluateRules(audience, null, {})).toBe(1); + expect(matcher.evaluateRules(audience, "production", {})).toEqual({ variant: 1 }); + expect(matcher.evaluateRules(audience, "staging", {})).toEqual({ variant: 1 }); + expect(matcher.evaluateRules(audience, null, {})).toEqual({ variant: 1 }); }); it("should skip rules when environments is non-empty and environment name is null", () => { @@ -148,7 +148,7 @@ describe("AudienceMatcher", () => { }, ], }); - expect(matcher.evaluateRules(audience, "production", { country: "US" })).toBe(1); + expect(matcher.evaluateRules(audience, "production", { country: "US" })).toEqual({ variant: 1 }); }); it("should return variant when conditions is null (matches all)", () => { @@ -163,7 +163,7 @@ describe("AudienceMatcher", () => { }, ], }); - expect(matcher.evaluateRules(audience, "production", {})).toBe(3); + expect(matcher.evaluateRules(audience, "production", {})).toEqual({ variant: 3 }); }); it("should return variant when conditions field is absent (matches all)", () => { @@ -177,7 +177,7 @@ describe("AudienceMatcher", () => { }, ], }); - expect(matcher.evaluateRules(audience, "production", {})).toBe(3); + expect(matcher.evaluateRules(audience, "production", {})).toEqual({ variant: 3 }); }); it("should handle malformed audience JSON gracefully", () => { @@ -229,7 +229,7 @@ describe("AudienceMatcher", () => { }, ], }); - expect(matcher.evaluateRules(audience, "production", {})).toBe(2); + expect(matcher.evaluateRules(audience, "production", {})).toEqual({ variant: 2 }); }); it("should skip rule with missing variant and continue to next valid rule", () => { @@ -248,7 +248,7 @@ describe("AudienceMatcher", () => { }, ], }); - expect(matcher.evaluateRules(audience, "production", {})).toBe(1); + expect(matcher.evaluateRules(audience, "production", {})).toEqual({ variant: 1 }); }); it("should handle malformed rules gracefully", () => { @@ -273,7 +273,7 @@ describe("AudienceMatcher", () => { }, ], }); - expect(matcher.evaluateRules(audience, "production", {})).toBe(2); + expect(matcher.evaluateRules(audience, "production", {})).toEqual({ variant: 2 }); }); it("should skip rules with missing type", () => { @@ -308,7 +308,7 @@ describe("AudienceMatcher", () => { }, ], }); - expect(matcher.evaluateRules(audience, "production", { country: "US" })).toBe(2); + expect(matcher.evaluateRules(audience, "production", { country: "US" })).toEqual({ variant: 2 }); }); it("should support variant 0", () => { @@ -323,7 +323,7 @@ describe("AudienceMatcher", () => { }, ], }); - expect(matcher.evaluateRules(audience, "production", {})).toBe(0); + expect(matcher.evaluateRules(audience, "production", {})).toEqual({ variant: 0 }); }); it("should skip rule with fractional variant", () => { @@ -343,7 +343,7 @@ describe("AudienceMatcher", () => { }, ], }); - expect(matcher.evaluateRules(audience, "production", {})).toBe(2); + expect(matcher.evaluateRules(audience, "production", {})).toEqual({ variant: 2 }); }); it("should skip rule with non-object conditions", () => { @@ -364,7 +364,7 @@ describe("AudienceMatcher", () => { }, ], }); - expect(matcher.evaluateRules(audience, "production", {})).toBe(2); + expect(matcher.evaluateRules(audience, "production", {})).toEqual({ variant: 2 }); }); it("should skip rule when environments is not an array", () => { @@ -415,7 +415,7 @@ describe("AudienceMatcher", () => { }, ], }); - expect(matcher.evaluateRules(audience, "production", {})).toBe(2); + expect(matcher.evaluateRules(audience, "production", {})).toEqual({ variant: 2 }); }); it("should return negative variant (bounds checking is caller responsibility)", () => { @@ -430,7 +430,49 @@ describe("AudienceMatcher", () => { }, ], }); - expect(matcher.evaluateRules(audience, "production", {})).toBe(-1); + expect(matcher.evaluateRules(audience, "production", {})).toEqual({ variant: -1 }); + }); + + describe("split rules", () => { + const splitRule = (percentages) => ({ + name: "split", + type: "split", + conditions: { and: [{ eq: [{ var: "country" }, { value: "US" }] }] }, + environments: [], + percentages, + }); + const fallbackRule = { name: "fallback", type: "assign", environments: [], variant: 2 }; + const evaluate = (rules) => + matcher.evaluateRules(JSON.stringify({ rules }), "production", { country: "US" }); + + it("should return the split as fractions when conditions match", () => { + expect(evaluate([splitRule("20/30/50")])).toEqual({ split: [0.2, 0.3, 0.5] }); + }); + + it("should accept a split whose sum is within the backend tolerance of 100", () => { + expect(evaluate([splitRule("33.33/33.33/33.33")])).toEqual({ split: [0.3333, 0.3333, 0.3333] }); + }); + + it("should return null when split conditions do not match", () => { + const audience = JSON.stringify({ rules: [splitRule("50/50")] }); + expect(matcher.evaluateRules(audience, "production", { country: "GB" })).toBe(null); + }); + + it("should skip a split rule with non-string percentages and continue to the next rule", () => { + expect(evaluate([splitRule([50, 50]), fallbackRule])).toEqual({ variant: 2 }); + }); + + it("should skip a split rule with non-numeric percentages and continue to the next rule", () => { + expect(evaluate([splitRule("50/abc"), fallbackRule])).toEqual({ variant: 2 }); + }); + + it("should skip a split rule with negative percentages and continue to the next rule", () => { + expect(evaluate([splitRule("150/-50"), fallbackRule])).toEqual({ variant: 2 }); + }); + + it("should skip a split rule whose percentages do not sum to 100 and continue to the next rule", () => { + expect(evaluate([splitRule("50/49"), fallbackRule])).toEqual({ variant: 2 }); + }); }); }); }); diff --git a/src/context.ts b/src/context.ts index 4040f57..9f5a8ba 100644 --- a/src/context.ts +++ b/src/context.ts @@ -356,20 +356,20 @@ export default class Context { this._invalidateAssignmentsPinnedWithMissingUnit(unitType); } - // A null holdout entry means the unit was missing when the assignment was resolved. Evict so - // `_assign()` rebuilds it; already-exposed ones are kept since re-resolving would queue a - // second, conflicting exposure. + // A null holdout entry, or a split rule override, may mean the unit was missing when the + // assignment was resolved. Evict so `_assign()` rebuilds it; already-exposed ones are kept since + // re-resolving would queue a second, conflicting exposure. private _invalidateAssignmentsPinnedWithMissingUnit(unitType: string): void { for (const experimentName in this._assignments) { const assignment = this._assignments[experimentName]; - const holdoutAssignments = assignment.holdoutAssignments; + if (assignment.unitType !== unitType || assignment.exposed) continue; - if (holdoutAssignments && assignment.unitType === unitType && !assignment.exposed) { - const hasMissingEntry = holdoutAssignments.some((holdoutAssignment) => holdoutAssignment === null); + const hasMissingHoldoutEntry = assignment.holdoutAssignments?.some( + (holdoutAssignment) => holdoutAssignment === null + ); - if (hasMissingEntry) { - delete this._assignments[experimentName]; - } + if (hasMissingHoldoutEntry || assignment.ruleOverride) { + delete this._assignments[experimentName]; } } } @@ -509,13 +509,21 @@ export default class Context { } } - private _computeRuleVariant( - assignmentRules: string, - variantCount: number, - attrs: Record - ): number | null { - const rawRuleVariant = this._audienceMatcher.evaluateRules(assignmentRules, this._environmentName, attrs); - return rawRuleVariant !== null && rawRuleVariant >= 0 && rawRuleVariant < variantCount ? rawRuleVariant : null; + private _computeRuleVariant(experiment: ExperimentData, attrs: Record): number | null { + const variantCount = experiment.variants.length; + const action = this._audienceMatcher.evaluateRules(experiment.assignmentRules ?? "", this._environmentName, attrs); + if (action == null) return null; + + if ("variant" in action) return action.variant >= 0 && action.variant < variantCount ? action.variant : null; + if (action.split.length !== variantCount) return null; + + const unitType = experiment.unitType; + const unit = unitType != null ? this._unitHash(unitType) : null; + if (unitType == null || unit === null) return 0; + + const assigner = + unitType in this._assigners ? this._assigners[unitType] : (this._assigners[unitType] = new VariantAssigner(unit)); + return assigner.assign(action.split, experiment.seedHi, experiment.seedLo); } private _checkReady(expectNotFinalized?: boolean) { @@ -590,7 +598,7 @@ export default class Context { const attrs = this._getAttributesMap(); if (experiment.assignmentRules && experiment.assignmentRules.length > 0) { - const ruleVariant = this._computeRuleVariant(experiment.assignmentRules, experiment.variants.length, attrs); + const ruleVariant = this._computeRuleVariant(experiment, attrs); if (ruleVariant !== (assignment.ruleVariant ?? null)) { return false; } @@ -730,11 +738,7 @@ export default class Context { let ruleVariant: number | null = null; if (experiment.data.assignmentRules && experiment.data.assignmentRules.length > 0) { - ruleVariant = this._computeRuleVariant( - experiment.data.assignmentRules, - experiment.data.variants.length, - attrs - ); + ruleVariant = this._computeRuleVariant(experiment.data, attrs); } assignment.ruleVariant = ruleVariant; diff --git a/src/matcher.ts b/src/matcher.ts index 47287e1..766e76a 100644 --- a/src/matcher.ts +++ b/src/matcher.ts @@ -1,6 +1,30 @@ import { isObject } from "./utils"; import { JsonExpr } from "./jsonexpr/jsonexpr"; +export type RuleAction = { variant: number } | { split: number[] }; + +const SPLIT_PERCENTAGES_SUM_TOLERANCE = 0.01; + +const parseSplitPercentages = (percentages: unknown) => { + if (typeof percentages !== "string") return null; + + const values = percentages.split("/").map((value) => parseFloat(value)); + if (values.some((value) => isNaN(value) || value < 0)) return null; + + const total = values.reduce((sum, value) => sum + value, 0); + if (parseFloat(Math.abs(100 - total).toFixed(2)) > SPLIT_PERCENTAGES_SUM_TOLERANCE) return null; + + return values.map((value) => value / 100); +}; + +const parseRuleAction = (rule: Record): RuleAction | null => { + if (rule.type === "assign") return Number.isInteger(rule.variant) ? { variant: rule.variant as number } : null; + if (rule.type !== "split") return null; + + const split = parseSplitPercentages(rule.percentages); + return split != null ? { split } : null; +}; + export class AudienceMatcher { evaluate(audienceString: string, vars: Record) { let audience; @@ -23,7 +47,7 @@ export class AudienceMatcher { assignmentRulesString: string, environmentName: string | null, vars: Record - ): number | null { + ): RuleAction | null { let assignmentRules; try { assignmentRules = JSON.parse(assignmentRulesString); @@ -37,7 +61,8 @@ export class AudienceMatcher { for (const rule of assignmentRules.rules) { if (!rule) continue; - if (rule.type !== "assign") continue; + const action = parseRuleAction(rule); + if (action == null) continue; if (rule.environments != null) { if (!Array.isArray(rule.environments)) continue; @@ -49,13 +74,10 @@ export class AudienceMatcher { } } - if (typeof rule.variant !== "number") continue; - if (rule.variant !== Math.floor(rule.variant)) continue; - const conditions = rule.conditions; if (conditions == null) { - return rule.variant; + return action; } if (!isObject(conditions)) continue; @@ -63,10 +85,10 @@ export class AudienceMatcher { try { const result = this._jsonExpr.evaluateBooleanExpr(conditions, vars); if (result === true) { - return rule.variant; + return action; } } catch (e) { - console.warn(`Failed to evaluate assignment rule conditions for variant ${rule.variant}: ${e}`); + console.warn(`Failed to evaluate assignment rule conditions for rule ${rule.name}: ${e}`); } } From 2175b37a2867423e35a2423c042a752e126de505 Mon Sep 17 00:00:00 2001 From: tarsis-viana Date: Thu, 8 Oct 2026 13:28:17 +0100 Subject: [PATCH 2/7] fix(FT-2287): only re-resolve split rules that matched without their unit Assign rules never need the unit, so evicting every unexposed rule override when a unit is set recomputed assign-rule assignments for nothing. Track whether the matched rule was a split resolved without its unit and evict only those. Co-Authored-By: Claude Opus 5.5 --- src/__tests__/context.test.js | 14 ++++++++++++++ src/context.ts | 33 +++++++++++++++++++-------------- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/src/__tests__/context.test.js b/src/__tests__/context.test.js index f035f33..145e32b 100644 --- a/src/__tests__/context.test.js +++ b/src/__tests__/context.test.js @@ -2998,6 +2998,20 @@ describe("Context", () => { context.unit("session_id", contextParams.units.session_id); expect(context.peek("exp_test_abc")).toEqual(1); }); + + it("should keep an assign rule's cached assignment when the unit is set later", () => { + const context = new Context( + sdk, + contextOptions, + { units: { user_id: contextParams.units.user_id } }, + rulesContextResponse + ); + context.attribute("country", "US"); + const assignment = context._assign("exp_test_abc"); + + context.unit("session_id", contextParams.units.session_id); + expect(context._assign("exp_test_abc")).toBe(assignment); + }); }); }); diff --git a/src/context.ts b/src/context.ts index 9f5a8ba..63f2953 100644 --- a/src/context.ts +++ b/src/context.ts @@ -65,6 +65,7 @@ type Assignment = { audienceMismatch: boolean; ruleOverride: boolean; ruleVariant?: number | null; + isRuleMissingUnit?: boolean; ruleKey?: string; trafficSplit?: number[]; variables?: Record; @@ -356,9 +357,9 @@ export default class Context { this._invalidateAssignmentsPinnedWithMissingUnit(unitType); } - // A null holdout entry, or a split rule override, may mean the unit was missing when the - // assignment was resolved. Evict so `_assign()` rebuilds it; already-exposed ones are kept since - // re-resolving would queue a second, conflicting exposure. + // A null holdout entry or a split rule resolved without its unit means the unit was missing when + // the assignment was resolved. Evict so `_assign()` rebuilds it; already-exposed ones are kept + // since re-resolving would queue a second, conflicting exposure. private _invalidateAssignmentsPinnedWithMissingUnit(unitType: string): void { for (const experimentName in this._assignments) { const assignment = this._assignments[experimentName]; @@ -368,7 +369,7 @@ export default class Context { (holdoutAssignment) => holdoutAssignment === null ); - if (hasMissingHoldoutEntry || assignment.ruleOverride) { + if (hasMissingHoldoutEntry || assignment.isRuleMissingUnit) { delete this._assignments[experimentName]; } } @@ -509,21 +510,24 @@ export default class Context { } } - private _computeRuleVariant(experiment: ExperimentData, attrs: Record): number | null { + private _resolveRule(experiment: ExperimentData, attrs: Record) { const variantCount = experiment.variants.length; const action = this._audienceMatcher.evaluateRules(experiment.assignmentRules ?? "", this._environmentName, attrs); if (action == null) return null; - if ("variant" in action) return action.variant >= 0 && action.variant < variantCount ? action.variant : null; + if ("variant" in action) { + const isInBounds = action.variant >= 0 && action.variant < variantCount; + return isInBounds ? { variant: action.variant, isMissingUnit: false } : null; + } if (action.split.length !== variantCount) return null; const unitType = experiment.unitType; const unit = unitType != null ? this._unitHash(unitType) : null; - if (unitType == null || unit === null) return 0; + if (unitType == null || unit === null) return { variant: 0, isMissingUnit: true }; const assigner = unitType in this._assigners ? this._assigners[unitType] : (this._assigners[unitType] = new VariantAssigner(unit)); - return assigner.assign(action.split, experiment.seedHi, experiment.seedLo); + return { variant: assigner.assign(action.split, experiment.seedHi, experiment.seedLo), isMissingUnit: false }; } private _checkReady(expectNotFinalized?: boolean) { @@ -598,7 +602,7 @@ export default class Context { const attrs = this._getAttributesMap(); if (experiment.assignmentRules && experiment.assignmentRules.length > 0) { - const ruleVariant = this._computeRuleVariant(experiment, attrs); + const ruleVariant = this._resolveRule(experiment, attrs)?.variant ?? null; if (ruleVariant !== (assignment.ruleVariant ?? null)) { return false; } @@ -735,13 +739,14 @@ export default class Context { ? `${experiment.data.assignmentRules}:${this._environmentName}` : ""; - let ruleVariant: number | null = null; - - if (experiment.data.assignmentRules && experiment.data.assignmentRules.length > 0) { - ruleVariant = this._computeRuleVariant(experiment.data, attrs); - } + const rule = + experiment.data.assignmentRules && experiment.data.assignmentRules.length > 0 + ? this._resolveRule(experiment.data, attrs) + : null; + const ruleVariant = rule?.variant ?? null; assignment.ruleVariant = ruleVariant; + assignment.isRuleMissingUnit = rule?.isMissingUnit ?? false; // A matching assignment rule wins over holdout suppression, the same way override() // does: it's an explicit, author-specified assignment (flagged `ruleOverride`, From 84fc3215608a94bd7bc75f9e373976fb03512d88 Mon Sep 17 00:00:00 2001 From: tarsis-viana Date: Thu, 8 Oct 2026 15:11:56 +0100 Subject: [PATCH 3/7] test(FT-2287): pin the split percentages sum tolerance at its boundaries The sum check rounds the gap from 100 to two decimals before comparing it with 0.01, the same check the backend applies to an experiment's own split, so gaps up to 0.0149... are accepted. Pin both sides of that boundary. Co-Authored-By: Claude Opus 5.5 --- src/__tests__/matcher.test.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/__tests__/matcher.test.js b/src/__tests__/matcher.test.js index a6b458c..f24fd1c 100644 --- a/src/__tests__/matcher.test.js +++ b/src/__tests__/matcher.test.js @@ -473,6 +473,21 @@ describe("AudienceMatcher", () => { it("should skip a split rule whose percentages do not sum to 100 and continue to the next rule", () => { expect(evaluate([splitRule("50/49"), fallbackRule])).toEqual({ variant: 2 }); }); + + // Same check as the backend's experiment split: the gap from 100 is rounded to two + // decimals before being compared with 0.01, so gaps up to 0.0149... are accepted. + it.each(["50/50.014", "50/49.986"])( + "should accept %s, whose gap from 100 rounds to 0.01", + (percentages) => { + expect(evaluate([splitRule(percentages), fallbackRule])).toEqual({ + split: percentages.split("/").map((value) => parseFloat(value) / 100), + }); + } + ); + + it.each(["50/50.015", "50/49.985"])("should skip %s, whose gap from 100 rounds to 0.02", (percentages) => { + expect(evaluate([splitRule(percentages), fallbackRule])).toEqual({ variant: 2 }); + }); }); }); }); From 01f08993fc3c869a5cd0eea4526d5a25ffb62922 Mon Sep 17 00:00:00 2001 From: tarsis-viana Date: Fri, 9 Oct 2026 11:51:33 +0100 Subject: [PATCH 4/7] fix(FT-2287): keep split rules resolved without their unit consistent The assignment cache check re-resolved split rules on every attribute change but only compared the variant, which caused two problems: - Its missing-unit flag went stale when a split began matching on the same variant as the previous rule, so setting the unit later never re-resolved it. - Once the unit was set, an exposed split that had fallen back to variant 0 re-hashed to its real variant on the next attribute change, switching the user and queueing a second exposure. The cache check now refreshes the flag, and keeps resolving an exposed missing-unit split as if the unit were still absent. Also tighten tests the review showed could pass without the behaviour they name: pick a split where experiment-seed hashing, traffic-seed hashing and normal assignment disagree; make the condition-error test actually throw; and pin that a split with the wrong value count stops rule evaluation. Co-Authored-By: Claude Opus 5.5 --- src/__tests__/context.test.js | 76 ++++++++++++++++++++++++++++++----- src/__tests__/matcher.test.js | 9 ++++- src/context.ts | 13 ++++-- 3 files changed, 82 insertions(+), 16 deletions(-) diff --git a/src/__tests__/context.test.js b/src/__tests__/context.test.js index 145e32b..7cf903d 100644 --- a/src/__tests__/context.test.js +++ b/src/__tests__/context.test.js @@ -2918,16 +2918,12 @@ describe("Context", () => { }), }); - it("should hash the unit with the experiment seed, so the experiment's own split keeps its variant", () => { - const context = new Context(sdk, contextOptions, contextParams, splitRulesResponse("34/33/33")); + // With these seeds the unit hashes to ~0.748 (experiment seed) and ~0.682 (traffic seed), and + // normal assignment gives variant 2, so each way of resolving the split lands on a different variant. + it("should hash the unit with the experiment seed", () => { + const context = new Context(sdk, contextOptions, contextParams, splitRulesResponse("70/10/20")); context.attribute("country", "US"); - expect(context.treatment("exp_test_abc")).toEqual(expectedVariants["exp_test_abc"]); - }); - - it("should not hash the unit with the traffic seed", () => { - const context = new Context(sdk, contextOptions, contextParams, splitRulesResponse("0/70/30")); - context.attribute("country", "US"); - expect(context.treatment("exp_test_abc")).toEqual(2); + expect(context.treatment("exp_test_abc")).toEqual(1); }); it("should assign the only variant with a non-zero share", () => { @@ -2972,8 +2968,20 @@ describe("Context", () => { expect(context.treatment("exp_test_abc")).toEqual(1); }); - it("should fall back to normal assignment when the split does not cover every variant", () => { - const context = new Context(sdk, contextOptions, contextParams, splitRulesResponse("0/100")); + it("should fall back to normal assignment without trying later rules when the split does not cover every variant", () => { + const laterAssignRule = { + name: "Everyone", + type: "assign", + conditions: null, + environments: [], + variant: 1, + }; + const context = new Context( + sdk, + contextOptions, + contextParams, + splitRulesResponse("0/100", [laterAssignRule]) + ); context.attribute("country", "US"); expect(context.treatment("exp_test_abc")).toEqual(expectedVariants["exp_test_abc"]); }); @@ -3012,6 +3020,52 @@ describe("Context", () => { context.unit("session_id", contextParams.units.session_id); expect(context._assign("exp_test_abc")).toBe(assignment); }); + + it("should re-resolve once the unit is set when a split rule starts matching on the same variant as the previous rule", () => { + const fallbackRule = { + name: "Everyone", + type: "assign", + conditions: null, + environments: [], + variant: 0, + }; + const context = new Context( + sdk, + contextOptions, + { units: { user_id: contextParams.units.user_id } }, + splitRulesResponse("0/100/0", [fallbackRule]) + ); + expect(context.peek("exp_test_abc")).toEqual(0); + + context.attribute("country", "US"); + expect(context.peek("exp_test_abc")).toEqual(0); + + context.unit("session_id", contextParams.units.session_id); + expect(context.peek("exp_test_abc")).toEqual(1); + }); + + it("should keep an exposed split on variant 0 after its unit is set, and expose it only once", (done) => { + const context = new Context( + sdk, + contextOptions, + { units: { user_id: contextParams.units.user_id } }, + splitRulesResponse("0/100/0") + ); + context.attribute("country", "US"); + expect(context.treatment("exp_test_abc")).toEqual(0); + + context.unit("session_id", contextParams.units.session_id); + context.attribute("unrelated", true); + expect(context.treatment("exp_test_abc")).toEqual(0); + + publisher.publish.mockReturnValue(Promise.resolve()); + + context.publish().then(() => { + const exposures = publisher.publish.mock.calls[0][0].exposures; + expect(exposures.filter((e) => e.name === "exp_test_abc").map((e) => e.variant)).toEqual([0]); + done(); + }); + }); }); }); diff --git a/src/__tests__/matcher.test.js b/src/__tests__/matcher.test.js index f24fd1c..1dc9ac9 100644 --- a/src/__tests__/matcher.test.js +++ b/src/__tests__/matcher.test.js @@ -398,12 +398,17 @@ describe("AudienceMatcher", () => { }); it("should skip rule when conditions evaluation throws and continue to next rule", () => { + const evaluateBooleanExpr = jest + .spyOn(matcher._jsonExpr, "evaluateBooleanExpr") + .mockImplementationOnce(() => { + throw new Error("condition failed"); + }); const audience = JSON.stringify({ rules: [ { name: "throws", type: "assign", - conditions: { badOperator: [1, 2] }, + conditions: { and: [{ value: true }] }, environments: [], variant: 1, }, @@ -416,6 +421,8 @@ describe("AudienceMatcher", () => { ], }); expect(matcher.evaluateRules(audience, "production", {})).toEqual({ variant: 2 }); + expect(evaluateBooleanExpr).toHaveBeenCalledTimes(1); + evaluateBooleanExpr.mockRestore(); }); it("should return negative variant (bounds checking is caller responsibility)", () => { diff --git a/src/context.ts b/src/context.ts index 63f2953..1c986a8 100644 --- a/src/context.ts +++ b/src/context.ts @@ -510,7 +510,7 @@ export default class Context { } } - private _resolveRule(experiment: ExperimentData, attrs: Record) { + private _resolveRule(experiment: ExperimentData, attrs: Record, isUnitPinnedMissing: boolean) { const variantCount = experiment.variants.length; const action = this._audienceMatcher.evaluateRules(experiment.assignmentRules ?? "", this._environmentName, attrs); if (action == null) return null; @@ -522,7 +522,7 @@ export default class Context { if (action.split.length !== variantCount) return null; const unitType = experiment.unitType; - const unit = unitType != null ? this._unitHash(unitType) : null; + const unit = unitType != null && !isUnitPinnedMissing ? this._unitHash(unitType) : null; if (unitType == null || unit === null) return { variant: 0, isMissingUnit: true }; const assigner = @@ -602,12 +602,17 @@ export default class Context { const attrs = this._getAttributesMap(); if (experiment.assignmentRules && experiment.assignmentRules.length > 0) { - const ruleVariant = this._resolveRule(experiment, attrs)?.variant ?? null; + // An exposed split resolved without its unit keeps resolving as if the unit were still + // missing, so a unit set afterwards cannot switch the variant the user already saw. + const isUnitPinnedMissing = assignment.exposed && assignment.isRuleMissingUnit === true; + const rule = this._resolveRule(experiment, attrs, isUnitPinnedMissing); + const ruleVariant = rule?.variant ?? null; if (ruleVariant !== (assignment.ruleVariant ?? null)) { return false; } assignment.ruleVariant = ruleVariant; + assignment.isRuleMissingUnit = rule?.isMissingUnit ?? false; } if (!assignment.ruleOverride && experiment.audience && experiment.audience.length > 0) { @@ -741,7 +746,7 @@ export default class Context { const rule = experiment.data.assignmentRules && experiment.data.assignmentRules.length > 0 - ? this._resolveRule(experiment.data, attrs) + ? this._resolveRule(experiment.data, attrs, false) : null; const ruleVariant = rule?.variant ?? null; From 4aab77f3fa6a281b9727531832350b853c141f3c Mon Sep 17 00:00:00 2001 From: tarsis-viana Date: Fri, 9 Oct 2026 13:36:23 +0100 Subject: [PATCH 5/7] fix(FT-2287): give the full-on variant when a split rule matches without its unit Without its unit a matched split rule fell back to variant 0, while a user who matched no rule on a full-on experiment got the full-on variant, so the rule made the outcome worse than having no rule. Fall back to the experiment's own no-unit result instead: the full-on variant, or 0 when the experiment is not full-on. Co-Authored-By: Claude Opus 5.5 --- src/__tests__/context.test.js | 27 +++++++++++++++++++++++++++ src/context.ts | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/__tests__/context.test.js b/src/__tests__/context.test.js index 7cf903d..0959085 100644 --- a/src/__tests__/context.test.js +++ b/src/__tests__/context.test.js @@ -3066,6 +3066,33 @@ describe("Context", () => { done(); }); }); + + it("should give the full-on variant while the unit is missing on a full-on experiment", () => { + const fullOnResponse = buildRulesResponse({ + fullOnVariant: 2, + assignmentRules: JSON.stringify({ + rules: [ + { + name: "Everyone", + type: "split", + conditions: null, + environments: [], + percentages: "0/100/0", + }, + ], + }), + }); + const context = new Context( + sdk, + contextOptions, + { units: { user_id: contextParams.units.user_id } }, + fullOnResponse + ); + expect(context.peek("exp_test_abc")).toEqual(2); + + context.unit("session_id", contextParams.units.session_id); + expect(context.peek("exp_test_abc")).toEqual(1); + }); }); }); diff --git a/src/context.ts b/src/context.ts index 1c986a8..b134495 100644 --- a/src/context.ts +++ b/src/context.ts @@ -523,7 +523,7 @@ export default class Context { const unitType = experiment.unitType; const unit = unitType != null && !isUnitPinnedMissing ? this._unitHash(unitType) : null; - if (unitType == null || unit === null) return { variant: 0, isMissingUnit: true }; + if (unitType == null || unit === null) return { variant: experiment.fullOnVariant, isMissingUnit: true }; const assigner = unitType in this._assigners ? this._assigners[unitType] : (this._assigners[unitType] = new VariantAssigner(unit)); From 94fa5b849d3b376987c420bbff1c5df214065be1 Mon Sep 17 00:00:00 2001 From: tarsis-viana Date: Fri, 9 Oct 2026 14:52:52 +0100 Subject: [PATCH 6/7] fix(FT-2287): keep a rule-won assignment cached when a custom assignment differs A custom assignment that lost to a matching rule made every treatment() call rebuild the assignment, resetting `exposed` and queueing duplicate exposures (also on main), and skipping the exposed missing-unit split pin. Co-Authored-By: Claude Opus 5.5 --- src/__tests__/context.test.js | 35 +++++++++++++++++++++++++++++++++++ src/context.ts | 10 ++++++++-- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/__tests__/context.test.js b/src/__tests__/context.test.js index 0959085..45113c0 100644 --- a/src/__tests__/context.test.js +++ b/src/__tests__/context.test.js @@ -2322,6 +2322,18 @@ describe("Context", () => { expect(context.treatment("exp_test_abc")).toEqual(1); }); + it("should expose a rule that wins over a custom assignment only once", () => { + const context = new Context(sdk, contextOptions, contextParams, rulesContextResponse); + context.attribute("country", "US"); + context.customAssignment("exp_test_abc", 2); + expect(context.treatment("exp_test_abc")).toEqual(1); + expect(context.treatment("exp_test_abc")).toEqual(1); + expect(context.pending()).toEqual(1); + + context.attribute("country", "GB"); + expect(context.treatment("exp_test_abc")).toEqual(2); + }); + it("should return normal assignment when no rules match", () => { const context = new Context(sdk, contextOptions, contextParams, rulesContextResponse); context.attribute("country", "GB"); @@ -3067,6 +3079,29 @@ describe("Context", () => { }); }); + it("should keep an exposed split on variant 0 after its unit is set when a custom assignment lost to it", (done) => { + const context = new Context( + sdk, + contextOptions, + { units: { user_id: contextParams.units.user_id } }, + splitRulesResponse("0/100/0") + ); + context.attribute("country", "US"); + context.customAssignment("exp_test_abc", 2); + expect(context.treatment("exp_test_abc")).toEqual(0); + + context.unit("session_id", contextParams.units.session_id); + expect(context.treatment("exp_test_abc")).toEqual(0); + + publisher.publish.mockReturnValue(Promise.resolve()); + + context.publish().then(() => { + const exposures = publisher.publish.mock.calls[0][0].exposures; + expect(exposures.filter((e) => e.name === "exp_test_abc").map((e) => e.variant)).toEqual([0]); + done(); + }); + }); + it("should give the full-on variant while the unit is missing on a full-on experiment", () => { const fullOnResponse = buildRulesResponse({ fullOnVariant: 2, diff --git a/src/context.ts b/src/context.ts index b134495..b45e94d 100644 --- a/src/context.ts +++ b/src/context.ts @@ -676,8 +676,14 @@ export default class Context { // previously not-running experiment return assignment; } - } else if (assignment.suppressed || !hasCustom || this._cassignments[experimentName] === assignment.variant) { - // Suppressed assignments are forced to variant 0, so a custom mismatch is not staleness. + } else if ( + assignment.suppressed || + assignment.ruleOverride || + !hasCustom || + this._cassignments[experimentName] === assignment.variant + ) { + // Suppressed assignments are forced to variant 0 and a matching rule wins over a custom + // assignment, so a custom mismatch is not staleness. if ( experimentMatches(experiment.data, assignment) && audienceMatches(experiment.data, assignment) && From 2c82256f3189bb674e8e86ac6e2f8810e5ea6cbb Mon Sep 17 00:00:00 2001 From: tarsis-viana Date: Fri, 9 Oct 2026 14:58:04 +0100 Subject: [PATCH 7/7] refactor(FT-2287): dispatch rule actions on type with a switch Makes the unknown-type fallback an explicit default and leaves one place to add further rule types. Co-Authored-By: Claude Opus 5.5 --- src/matcher.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/matcher.ts b/src/matcher.ts index 766e76a..706fd99 100644 --- a/src/matcher.ts +++ b/src/matcher.ts @@ -18,11 +18,16 @@ const parseSplitPercentages = (percentages: unknown) => { }; const parseRuleAction = (rule: Record): RuleAction | null => { - if (rule.type === "assign") return Number.isInteger(rule.variant) ? { variant: rule.variant as number } : null; - if (rule.type !== "split") return null; - - const split = parseSplitPercentages(rule.percentages); - return split != null ? { split } : null; + switch (rule.type) { + case "assign": + return Number.isInteger(rule.variant) ? { variant: rule.variant as number } : null; + case "split": { + const split = parseSplitPercentages(rule.percentages); + return split != null ? { split } : null; + } + default: + return null; + } }; export class AudienceMatcher {