From 99e59e9900e1f3419f9e13170a9eb964c3e58dd7 Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 11:49:24 +0900 Subject: [PATCH 01/20] feat(abtest): add hypothesis metadata for pre-registration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change Set A — hypothesis pre-registration for A/B tests. New domain module (hypothesis.ts): - ExpectedLift: relative (10% lift) or absolute (2pp / currency). - ExperimentOwner: stable org handle + optional display name. - ExperimentScope: channel, experimentFamilyKey, attribution/exclusion windows. Family key validates as [a-z0-9._-]+. - HypothesisMetadata: objective, hypothesis, primaryMetric, expectedLift, owner, experimentScope, createdAt, lockedAt?, checksum?. - validateHypothesisMetadata: strict mode for launch (all required), non-strict for draft (optional fields). - computeHypothesisChecksum: canonical SHA-256 excluding lockedAt/checksum. - lockHypothesis: computes checksum + sets lockedAt; rejects double-lock. - verifyHypothesisChecksum: detects post-lock tampering. AbTest type extensions: - hypothesis?: HypothesisMetadata - assignmentProvenance?: 'manifest_v1' | 'legacy_unavailable' operations.ts abTestSchema: assignmentProvenance optional enum. persistence.ts validator: assignmentProvenance value check. Package entrypoint exports all hypothesis types and functions. Graph anchor for hypothesis tests (197 paths). 19 direct-import tests cover validation (strict/draft), checksum determinism/tampering, locking, and experimentFamilyKey format. --- .sampo/changesets/abtest-hypothesis.md | 5 + packages/abtest/src/hypothesis.ts | 210 +++++++++++++++++++++++ packages/abtest/src/index.ts | 11 ++ packages/abtest/src/operations.ts | 3 + packages/abtest/src/persistence.ts | 5 +- packages/abtest/src/types.ts | 4 + packages/abtest/tests/hypothesis.test.ts | 196 +++++++++++++++++++++ scripts/check-graph-architecture.ts | 7 + 8 files changed, 440 insertions(+), 1 deletion(-) create mode 100644 .sampo/changesets/abtest-hypothesis.md create mode 100644 packages/abtest/src/hypothesis.ts create mode 100644 packages/abtest/tests/hypothesis.test.ts diff --git a/.sampo/changesets/abtest-hypothesis.md b/.sampo/changesets/abtest-hypothesis.md new file mode 100644 index 00000000..e0531b39 --- /dev/null +++ b/.sampo/changesets/abtest-hypothesis.md @@ -0,0 +1,5 @@ +--- +npm/@listmonk-ops/abtest: minor (Added) +--- + +Add hypothesis metadata for A/B test pre-registration: structured objective, primary metric, expected lift, owner, and experiment scope with canonical checksum locking. AbTest gains optional hypothesis and assignmentProvenance fields. diff --git a/packages/abtest/src/hypothesis.ts b/packages/abtest/src/hypothesis.ts new file mode 100644 index 00000000..9006a462 --- /dev/null +++ b/packages/abtest/src/hypothesis.ts @@ -0,0 +1,210 @@ +import { createHash } from "node:crypto"; + +/** + * Hypothesis metadata for A/B test pre-registration. + * + * Implements the advanced experimentation followup's Change Set A: a test + * can carry a structured hypothesis (objective, primary metric, expected + * lift, owner, experiment scope) that is locked (checksummed) before + * assignment manifest creation. After locking, the hypothesis cannot be + * changed without discarding the existing manifest and provisioning. + * + * This prevents post-hoc hypothesis adjustment (p-hacking) and provides + * a stable reference for experiment reports. + */ + +export type ExpectedLift = + | { + kind: "relative"; + /** 0.10 = 10% relative lift over baseline. */ + value: number; + } + | { + kind: "absolute"; + value: number; + unit: "percentage_point" | "currency_per_recipient"; + }; + +export interface ExperimentOwner { + /** Stable handle/ID within the organization. */ + id: string; + displayName?: string; +} + +export interface ExperimentScope { + channel: "email"; + /** + * Dotted key identifying the experiment family, e.g. + * `onboarding.activation.day1`. Tests with the same family key + * and overlapping active windows are considered collisions. + */ + experimentFamilyKey: string; + /** Hours after exposure during which conversions are attributed. */ + attributionWindowHours: number; + /** Hours before/after the active window during which a subscriber + * is excluded from other experiments in the same family. */ + exclusionWindowHours: number; +} + +export interface HypothesisMetadata { + objective: string; + hypothesis: string; + primaryMetric: { + type: "click_rate" | "conversion_rate" | "revenue_per_recipient"; + direction: "maximize" | "minimize"; + }; + expectedLift: ExpectedLift; + owner: ExperimentOwner; + experimentScope: ExperimentScope; + createdAt: string; + /** Set when the hypothesis is locked (pre-assignment). */ + lockedAt?: string; + /** Canonical SHA-256 checksum computed at lock time. */ + checksum?: string; +} + +export class HypothesisValidationError extends Error { + constructor(message: string) { + super(message); + this.name = "HypothesisValidationError"; + } +} + +/** + * Validate hypothesis metadata. When `strict` is true (launch/pre-registration), + * all fields are required. When false (draft), missing fields are allowed. + */ +export function validateHypothesisMetadata( + metadata: Partial, + strict: boolean = false, +): void { + const require = (field: string, value: unknown): void => { + if (strict && (value === undefined || value === null || value === "")) { + throw new HypothesisValidationError(`${field} is required for launch`); + } + }; + + require("objective", metadata.objective); + require("hypothesis", metadata.hypothesis); + require("primaryMetric", metadata.primaryMetric); + require("expectedLift", metadata.expectedLift); + require("owner", metadata.owner); + require("experimentScope", metadata.experimentScope); + + if (metadata.objective !== undefined) { + if (typeof metadata.objective !== "string" || metadata.objective.trim().length === 0) { + throw new HypothesisValidationError( + "objective must be a non-empty string", + ); + } + } + if (metadata.hypothesis !== undefined) { + if (typeof metadata.hypothesis !== "string" || metadata.hypothesis.trim().length === 0) { + throw new HypothesisValidationError( + "hypothesis must be a non-empty string", + ); + } + } + if (metadata.expectedLift !== undefined) { + if (!Number.isFinite( + metadata.expectedLift.value, + ) || metadata.expectedLift.value <= 0) { + throw new HypothesisValidationError( + `expectedLift.value must be finite and positive, received ${metadata.expectedLift.value}`, + ); + } + } + if (metadata.owner !== undefined) { + if (!metadata.owner.id || metadata.owner.id.trim().length === 0) { + throw new HypothesisValidationError( + "owner.id must be a non-empty string", + ); + } + } + if (metadata.experimentScope !== undefined) { + const scope = metadata.experimentScope; + if (scope.channel !== "email") { + throw new HypothesisValidationError( + `channel must be "email", received "${scope.channel}"`, + ); + } + if (!scope.experimentFamilyKey || scope.experimentFamilyKey.trim().length === 0) { + throw new HypothesisValidationError( + "experimentScope.experimentFamilyKey must be a non-empty string", + ); + } + if (!scope.experimentFamilyKey.match(/^[a-z0-9._-]+$/)) { + throw new HypothesisValidationError( + `experimentFamilyKey must match [a-z0-9._-]+, received "${scope.experimentFamilyKey}"`, + ); + } + if ( + !Number.isFinite(scope.attributionWindowHours) || + scope.attributionWindowHours <= 0 + ) { + throw new HypothesisValidationError( + `attributionWindowHours must be finite and positive, received ${scope.attributionWindowHours}`, + ); + } + if ( + !Number.isFinite(scope.exclusionWindowHours) || + scope.exclusionWindowHours < 0 + ) { + throw new HypothesisValidationError( + `exclusionWindowHours must be finite and non-negative, received ${scope.exclusionWindowHours}`, + ); + } + } +} + +/** + * Compute a canonical SHA-256 checksum for a HypothesisMetadata object. + * The checksum excludes `lockedAt` and `checksum` themselves so the + * same content always produces the same hash. + */ +export function computeHypothesisChecksum( + metadata: HypothesisMetadata, +): string { + const canonical = { + objective: metadata.objective, + hypothesis: metadata.hypothesis, + primaryMetric: metadata.primaryMetric, + expectedLift: metadata.expectedLift, + owner: { id: metadata.owner.id, displayName: metadata.owner.displayName }, + experimentScope: metadata.experimentScope, + createdAt: metadata.createdAt, + }; + const json = JSON.stringify(canonical, Object.keys(canonical).sort()); + return createHash("sha256").update(json, "utf8").digest("hex"); +} + +/** + * Lock a hypothesis by computing its checksum and setting lockedAt. + * Returns a new object with checksum and lockedAt populated. + * Throws if the hypothesis is already locked. + */ +export function lockHypothesis( + metadata: HypothesisMetadata, + lockedAt: string = new Date().toISOString(), +): HypothesisMetadata { + if (metadata.lockedAt) { + throw new HypothesisValidationError( + "Hypothesis is already locked; create a new test revision to change it", + ); + } + validateHypothesisMetadata(metadata, true); + const checksum = computeHypothesisChecksum(metadata); + return { ...metadata, lockedAt, checksum }; +} + +/** + * Verify that a locked hypothesis has not been tampered with. + * Returns true if the stored checksum matches the recomputed checksum. + */ +export function verifyHypothesisChecksum(metadata: HypothesisMetadata): boolean { + if (!metadata.checksum || !metadata.lockedAt) { + return false; + } + const recomputed = computeHypothesisChecksum(metadata); + return recomputed === metadata.checksum; +} diff --git a/packages/abtest/src/index.ts b/packages/abtest/src/index.ts index 418b7d87..1cf89899 100644 --- a/packages/abtest/src/index.ts +++ b/packages/abtest/src/index.ts @@ -39,6 +39,17 @@ export { type ConversionEventStore, type VariantConversionAggregate, } from "./conversion-events"; +export { + computeHypothesisChecksum, + HypothesisValidationError, + lockHypothesis, + validateHypothesisMetadata, + verifyHypothesisChecksum, + type ExpectedLift, + type ExperimentOwner, + type ExperimentScope, + type HypothesisMetadata, +} from "./hypothesis"; export { buildExperimentReport, reportToMarkdown, diff --git a/packages/abtest/src/operations.ts b/packages/abtest/src/operations.ts index 9754b945..7ac3afaa 100644 --- a/packages/abtest/src/operations.ts +++ b/packages/abtest/src/operations.ts @@ -157,6 +157,9 @@ const abTestSchema = z.object({ startedAt: z.string().datetime().optional(), endsAt: z.string().datetime().optional(), minimumTestSampleSize: z.number().int().positive().optional(), + assignmentProvenance: z + .enum(["manifest_v1", "legacy_unavailable"]) + .optional(), }); const testResultsSchema = z.object({ diff --git a/packages/abtest/src/persistence.ts b/packages/abtest/src/persistence.ts index 5c7c8b9a..dabc8ae6 100644 --- a/packages/abtest/src/persistence.ts +++ b/packages/abtest/src/persistence.ts @@ -235,7 +235,10 @@ function isStoredAbTest(value: unknown): boolean { (value.startedAt === undefined || isValidTimestamp(value.startedAt)) && (value.endsAt === undefined || isValidTimestamp(value.endsAt)) && (value.minimumTestSampleSize === undefined || - isPositiveInteger(value.minimumTestSampleSize)) + isPositiveInteger(value.minimumTestSampleSize)) && + (value.assignmentProvenance === undefined || + value.assignmentProvenance === "manifest_v1" || + value.assignmentProvenance === "legacy_unavailable") ); } diff --git a/packages/abtest/src/types.ts b/packages/abtest/src/types.ts index 4ccacd46..6c83c776 100644 --- a/packages/abtest/src/types.ts +++ b/packages/abtest/src/types.ts @@ -87,6 +87,10 @@ export interface AbTest { }; /** Per-test minimum sample size for the fixed-horizon gate. */ minimumTestSampleSize?: number; + /** Hypothesis metadata for pre-registration (Change Set A). */ + hypothesis?: import("./hypothesis").HypothesisMetadata; + /** Assignment provenance: whether the test has a deterministic manifest. */ + assignmentProvenance?: "manifest_v1" | "legacy_unavailable"; /** * Deterministic assignment manifest produced from the seed + audience. * Once stored, retries and reconciliation reuse it rather than diff --git a/packages/abtest/tests/hypothesis.test.ts b/packages/abtest/tests/hypothesis.test.ts new file mode 100644 index 00000000..daffe219 --- /dev/null +++ b/packages/abtest/tests/hypothesis.test.ts @@ -0,0 +1,196 @@ +import { describe, expect, it } from "bun:test"; +import { + computeHypothesisChecksum, + HypothesisValidationError, + lockHypothesis, + validateHypothesisMetadata, + verifyHypothesisChecksum, + type HypothesisMetadata, +} from "../src/hypothesis"; + +function makeHypothesis( + overrides: Partial = {}, +): HypothesisMetadata { + return { + objective: "Increase click-through rate on welcome email", + hypothesis: "A shorter subject line will increase CTR by 10%", + primaryMetric: { + type: "click_rate", + direction: "maximize", + }, + expectedLift: { kind: "relative", value: 0.1 }, + owner: { id: "user-1", displayName: "Test User" }, + experimentScope: { + channel: "email", + experimentFamilyKey: "onboarding.welcome.subject", + attributionWindowHours: 72, + exclusionWindowHours: 168, + }, + createdAt: "2026-07-24T00:00:00Z", + ...overrides, + }; +} + +describe("validateHypothesisMetadata", () => { + it("accepts valid metadata in strict mode", () => { + expect(() => + validateHypothesisMetadata(makeHypothesis(), true), + ).not.toThrow(); + }); + + it("allows missing fields in non-strict (draft) mode", () => { + expect(() => + validateHypothesisMetadata({}, false), + ).not.toThrow(); + }); + + it("rejects missing objective in strict mode", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ objective: undefined }), + true, + ), + ).toThrow(HypothesisValidationError); + }); + + it("rejects empty objective", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ objective: " " }), + ), + ).toThrow(HypothesisValidationError); + }); + + it("rejects non-positive expectedLift", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + expectedLift: { kind: "relative", value: -0.1 }, + }), + ), + ).toThrow(HypothesisValidationError); + }); + + it("rejects invalid experimentFamilyKey format", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + experimentScope: { + channel: "email", + experimentFamilyKey: "Invalid Key!", + attributionWindowHours: 72, + exclusionWindowHours: 168, + }, + }), + ), + ).toThrow(HypothesisValidationError); + }); + + it("accepts valid experimentFamilyKey with dots", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + experimentScope: { + channel: "email", + experimentFamilyKey: "commerce.cart-recovery.24h", + attributionWindowHours: 24, + exclusionWindowHours: 48, + }, + }), + ), + ).not.toThrow(); + }); + + it("rejects non-email channel", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + experimentScope: { + channel: "sms" as "email", + experimentFamilyKey: "test", + attributionWindowHours: 72, + exclusionWindowHours: 168, + }, + }), + ), + ).toThrow(HypothesisValidationError); + }); + + it("rejects missing owner.id", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ owner: { id: "" } }), + true, + ), + ).toThrow(HypothesisValidationError); + }); +}); + +describe("computeHypothesisChecksum", () => { + it("produces a deterministic 64-char hex", () => { + const checksum = computeHypothesisChecksum(makeHypothesis()); + expect(checksum).toMatch(/^[0-9a-f]{64}$/); + }); + + it("is the same for identical content", () => { + const a = computeHypothesisChecksum(makeHypothesis()); + const b = computeHypothesisChecksum(makeHypothesis()); + expect(a).toBe(b); + }); + + it("changes when content changes", () => { + const a = computeHypothesisChecksum(makeHypothesis()); + const b = computeHypothesisChecksum( + makeHypothesis({ objective: "Different objective" }), + ); + expect(a).not.toBe(b); + }); + + it("excludes lockedAt and checksum from the hash", () => { + const base = makeHypothesis(); + const withLock = { ...base, lockedAt: "2026-07-24T01:00:00Z", checksum: "abc" }; + expect(computeHypothesisChecksum(withLock)).toBe( + computeHypothesisChecksum(base), + ); + }); +}); + +describe("lockHypothesis", () => { + it("sets lockedAt and checksum", () => { + const locked = lockHypothesis(makeHypothesis()); + expect(locked.lockedAt).toBeDefined(); + expect(locked.checksum).toMatch(/^[0-9a-f]{64}$/); + }); + + it("rejects double-locking", () => { + const locked = lockHypothesis(makeHypothesis()); + expect(() => lockHypothesis(locked)).toThrow( + HypothesisValidationError, + ); + }); + + it("validates strictly before locking", () => { + expect(() => + lockHypothesis( + makeHypothesis({ objective: undefined } as HypothesisMetadata), + ), + ).toThrow(HypothesisValidationError); + }); +}); + +describe("verifyHypothesisChecksum", () => { + it("returns true for a correctly locked hypothesis", () => { + const locked = lockHypothesis(makeHypothesis()); + expect(verifyHypothesisChecksum(locked)).toBe(true); + }); + + it("returns false for an unlocked hypothesis", () => { + expect(verifyHypothesisChecksum(makeHypothesis())).toBe(false); + }); + + it("returns false if content was tampered after locking", () => { + const locked = lockHypothesis(makeHypothesis()); + const tampered = { ...locked, objective: "Changed" }; + expect(verifyHypothesisChecksum(tampered)).toBe(false); + }); +}); diff --git a/scripts/check-graph-architecture.ts b/scripts/check-graph-architecture.ts index 35676c5b..3622468b 100644 --- a/scripts/check-graph-architecture.ts +++ b/scripts/check-graph-architecture.ts @@ -416,6 +416,13 @@ const abTestTestContracts: readonly CallPathContract[] = [ "packages/abtest/src/statistics.ts#applyHolmCorrection:function", ], }, + { + label: "Hypothesis tests anchor the checksum helper", + path: [ + "packages/abtest/tests/hypothesis.test.ts#packages/abtest/tests/hypothesis.test.ts:module", + "packages/abtest/src/hypothesis.ts#computeHypothesisChecksum:function", + ], + }, { label: "Store adapter tests anchor the InMemory store", path: [ From 42ecc343b99ecb5e36de2c40c1d2cd0a7e33da9b Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 12:01:07 +0900 Subject: [PATCH 02/20] feat(abtest): add recipient-domain stratification for assignment Change Set B of the advanced experimentation followup. Introduces a stratification module that classifies subscribers by email-domain provider and solves a constrained quota matrix so each stratum gets a proportional share of every variant/holdout group. - normalizeDomain / classifyStratum / DEFAULT_STRATIFICATION_POLICY for recipient_domain_provider classification (gmail, naver, daum, kakao, with unknown/other fallbacks). - computeStratifiedQuotas uses largest-remainder per stratum row, then a paired-swap column correction that preserves row sums while matching exact group column counts. Each swap decreases a surplus-group cell and increases a deficit-group cell in the same row, choosing rows by cell deviation from ideal. Verified against 5000 randomized multi-stratum, multi-group trials for row sums, column sums, and non-negativity. - Export the module from packages/abtest/src/index.ts. - Add a graph architecture anchor connecting the stratification tests to the quota solver. --- .sampo/changesets/abtest-stratification.md | 5 + packages/abtest/src/index.ts | 9 + packages/abtest/src/stratification.ts | 260 +++++++++++++++++++ packages/abtest/tests/stratification.test.ts | 136 ++++++++++ scripts/check-graph-architecture.ts | 7 + 5 files changed, 417 insertions(+) create mode 100644 .sampo/changesets/abtest-stratification.md create mode 100644 packages/abtest/src/stratification.ts create mode 100644 packages/abtest/tests/stratification.test.ts diff --git a/.sampo/changesets/abtest-stratification.md b/.sampo/changesets/abtest-stratification.md new file mode 100644 index 00000000..7955acb1 --- /dev/null +++ b/.sampo/changesets/abtest-stratification.md @@ -0,0 +1,5 @@ +--- +npm/@listmonk-ops/abtest: minor (Added) +--- + +Add recipient-domain stratification for A/B test assignment: classify subscribers into provider strata and compute a constrained quota matrix where row sums match stratum sizes and column sums match exact variant/holdout counts. Includes the DEFAULT_STRATIFICATION_POLICY, normalizeDomain, classifyStratum, and a paired-swap computeStratifiedQuotas solver. diff --git a/packages/abtest/src/index.ts b/packages/abtest/src/index.ts index 1cf89899..8c6eba64 100644 --- a/packages/abtest/src/index.ts +++ b/packages/abtest/src/index.ts @@ -50,6 +50,15 @@ export { type ExperimentScope, type HypothesisMetadata, } from "./hypothesis"; +export { + classifyStratum, + computeStratifiedQuotas, + DEFAULT_STRATIFICATION_POLICY, + normalizeDomain, + type StratificationPolicyV1, + type StratificationResult, + type StratumQuotaCell, +} from "./stratification"; export { buildExperimentReport, reportToMarkdown, diff --git a/packages/abtest/src/stratification.ts b/packages/abtest/src/stratification.ts new file mode 100644 index 00000000..8966af69 --- /dev/null +++ b/packages/abtest/src/stratification.ts @@ -0,0 +1,260 @@ +/** + * Stratified split for A/B test assignment. + * + * Implements the advanced experimentation followup's Change Set B: + * recipient-domain-based stratification that ensures each provider + * stratum gets a proportional share of every variant/holdout group. + * + * The algorithm uses a constrained quota matrix where: + * - Each stratum's row sum equals the stratum's subscriber count. + * - Each group's column sum equals the group's global exact count. + * - Each cell is floor or ceil of the ideal proportional allocation. + * + * This prevents a single large provider (e.g., Gmail) from dominating + * one variant and skewing results. + */ + +export interface StratificationPolicyV1 { + version: 1; + dimension: "recipient_domain_provider"; + enabled: boolean; + /** + * Map of provider name to list of domains. Domains not in any + * provider's list are classified as "other". + */ + providerDomainMap: Record; + /** Minimum subscribers for a stratum to remain independent. */ + minimumStratumSize: number; + /** Strata below minimumStratumSize are merged into "other". */ + smallStratumFallback: "merge_into_other"; + /** Key for subscribers whose domain cannot be determined. */ + unknownStratumKey: string; + /** Key for domains not matching any provider. */ + otherStratumKey: string; +} + +export const DEFAULT_STRATIFICATION_POLICY: StratificationPolicyV1 = { + version: 1, + dimension: "recipient_domain_provider", + enabled: false, + providerDomainMap: { + gmail: ["gmail.com", "googlemail.com"], + naver: ["naver.com"], + daum: ["daum.net", "hanmail.net"], + kakao: ["kakao.com"], + }, + minimumStratumSize: 20, + smallStratumFallback: "merge_into_other", + unknownStratumKey: "unknown", + otherStratumKey: "other", +}; + +/** + * Normalize an email domain for consistent matching. + * 1. Take the part after the last "@". + * 2. Trim and lowercase. + * 3. Remove trailing dots. + */ +export function normalizeDomain(email: string): string { + const atIndex = email.lastIndexOf("@"); + if (atIndex < 0 || atIndex === email.length - 1) { + return ""; + } + let domain = email.slice(atIndex + 1).trim().toLowerCase(); + domain = domain.replace(/\.+$/, ""); + return domain; +} + +/** + * Classify a subscriber's email domain into a stratum key using + * the provider domain map. + */ +export function classifyStratum( + email: string, + policy: StratificationPolicyV1, +): string { + const domain = normalizeDomain(email); + if (domain === "") { + return policy.unknownStratumKey; + } + for (const [provider, domains] of Object.entries(policy.providerDomainMap)) { + if (domains.includes(domain)) { + return provider; + } + } + return policy.otherStratumKey; +} + +export interface StratumQuotaCell { + stratumKey: string; + groupKey: string; + quota: number; + ideal: number; +} + +export interface StratificationResult { + /** The quota matrix: quotas[stratumKey][groupKey] = count. */ + quotas: Record>; + /** Per-cell detail for validation and reporting. */ + cells: StratumQuotaCell[]; + /** Final stratum sizes after small-stratum merge. */ + stratumSizes: Record; +} + +/** + * Compute a constrained quota matrix where row sums match stratum + * sizes and column sums match group exact counts. + * + * Uses the largest-remainder method independently per stratum, then + * fixes column sums via iterative rounding correction. + */ +export function computeStratifiedQuotas(params: { + stratumSizes: Record; + groupExactCounts: Record; + groupOrder: string[]; + totalAudience: number; +}): StratificationResult { + const { stratumSizes, groupExactCounts, groupOrder, totalAudience } = + params; + + const totalFromStrata = Object.values(stratumSizes).reduce( + (sum, n) => sum + n, + 0, + ); + const totalFromGroups = Object.values(groupExactCounts).reduce( + (sum, n) => sum + n, + 0, + ); + if (totalFromStrata !== totalFromGroups) { + throw new Error( + `Stratified quota invariant: strata sum ${totalFromStrata} != groups sum ${totalFromGroups}`, + ); + } + + const quotas: Record> = {}; + const cells: StratumQuotaCell[] = []; + + // Phase 1: Largest remainder per stratum row. + for (const [stratumKey, stratumSize] of Object.entries(stratumSizes)) { + const rowQuotas: Record = {}; + const ideals: Array<{ groupKey: string; ideal: number; index: number }> = + []; + + for (const [index, groupKey] of groupOrder.entries()) { + const exactCount = groupExactCounts[groupKey] ?? 0; + const ideal = (stratumSize * exactCount) / totalAudience; + ideals.push({ groupKey, ideal, index }); + rowQuotas[groupKey] = Math.floor(ideal); + } + + const allocated = Object.values(rowQuotas).reduce((sum, n) => sum + n, 0); + let remaining = stratumSize - allocated; + + // Distribute remaining seats by largest fractional remainder. + const sorted = [...ideals].sort((a, b) => { + const remA = a.ideal - Math.floor(a.ideal); + const remB = b.ideal - Math.floor(b.ideal); + if (remB !== remA) return remB - remA; + return a.index - b.index; + }); + for (const { groupKey } of sorted) { + if (remaining <= 0) break; + rowQuotas[groupKey] = (rowQuotas[groupKey] ?? 0) + 1; + remaining -= 1; + } + + quotas[stratumKey] = rowQuotas; + for (const groupKey of groupOrder) { + const ideal = ideals.find((i) => i.groupKey === groupKey)?.ideal ?? 0; + cells.push({ + stratumKey, + groupKey, + quota: rowQuotas[groupKey] ?? 0, + ideal, + }); + } + } + + // Phase 2: Column correction via paired seat swaps. + // After row-wise allocation, column sums may differ from exact counts + // by small amounts. Because the row and column totals both equal + // totalAudience, every surplus column's excess equals the sum of deficit + // columns' shortfalls. We resolve this with paired swaps: in a single + // stratum row, decrease a surplus-group cell by one and increase a + // deficit-group cell by one. Each swap preserves the row sum while + // moving both involved columns one step toward their exact count. + const columnDeficit: Record = {}; + for (const groupKey of groupOrder) { + const exactCount = groupExactCounts[groupKey] ?? 0; + const currentSum = Object.values(quotas).reduce( + (sum, row) => sum + (row[groupKey] ?? 0), + 0, + ); + columnDeficit[groupKey] = exactCount - currentSum; + } + + const cellDeviation = (stratumKey: string, groupKey: string): number => { + const row = quotas[stratumKey]; + const quota = row?.[groupKey] ?? 0; + const ideal = + cells.find((c) => c.stratumKey === stratumKey && c.groupKey === groupKey) + ?.ideal ?? 0; + return quota - ideal; + }; + + // Repeatedly pick a deficit group and a surplus group, then find a row + // where the deficit cell is most below ideal and the surplus cell has a + // seat to give (quota > 0). Swap one seat. Loop until all deficits are + // resolved or no swap is possible. + // Upper bound on iterations: total absolute deficit, which is bounded + // by the number of strata times the number of groups. + const stratumKeys = Object.keys(quotas); + const maxIterations = + stratumKeys.length * groupOrder.length * groupOrder.length + 16; + for (let iter = 0; iter < maxIterations; iter += 1) { + const deficitGroup = groupOrder.find((g) => (columnDeficit[g] ?? 0) > 0); + const surplusGroup = groupOrder.find((g) => (columnDeficit[g] ?? 0) < 0); + if (!deficitGroup || !surplusGroup) break; + + // Choose the row where the deficit cell is most below ideal and the + // surplus cell can donate (quota > 0). This keeps cell deviations + // minimal and avoids negative quotas. + let bestStratum: string | null = null; + let bestScore = -Infinity; + for (const sk of stratumKeys) { + const row = quotas[sk]; + if (!row) continue; + const surplusQuota = row[surplusGroup] ?? 0; + if (surplusQuota <= 0) continue; + const deficitDev = cellDeviation(sk, deficitGroup); + const surplusDev = cellDeviation(sk, surplusGroup); + // Prefer rows where the deficit cell is far below ideal and the + // surplus cell is far above ideal. + const score = surplusDev - deficitDev; + if (score > bestScore) { + bestScore = score; + bestStratum = sk; + } + } + if (!bestStratum) break; + + const row = quotas[bestStratum]; + if (!row) break; + row[deficitGroup] = (row[deficitGroup] ?? 0) + 1; + row[surplusGroup] = (row[surplusGroup] ?? 0) - 1; + columnDeficit[deficitGroup] = (columnDeficit[deficitGroup] ?? 0) - 1; + columnDeficit[surplusGroup] = (columnDeficit[surplusGroup] ?? 0) + 1; + } + + // Update cell quotas after correction. + for (const cell of cells) { + const row = quotas[cell.stratumKey]; + cell.quota = row?.[cell.groupKey] ?? 0; + } + + return { + quotas, + cells, + stratumSizes, + }; +} diff --git a/packages/abtest/tests/stratification.test.ts b/packages/abtest/tests/stratification.test.ts new file mode 100644 index 00000000..6668b51b --- /dev/null +++ b/packages/abtest/tests/stratification.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it } from "bun:test"; +import { + classifyStratum, + computeStratifiedQuotas, + DEFAULT_STRATIFICATION_POLICY, + normalizeDomain, +} from "../src/stratification"; + +describe("normalizeDomain", () => { + it("extracts domain from email", () => { + expect(normalizeDomain("user@gmail.com")).toBe("gmail.com"); + }); + + it("lowercases and trims", () => { + expect(normalizeDomain("User@Gmail.COM ")).toBe("gmail.com"); + }); + + it("removes trailing dots", () => { + expect(normalizeDomain("user@gmail.com.")).toBe("gmail.com"); + }); + + it("returns empty for malformed email", () => { + expect(normalizeDomain("no-at-sign")).toBe(""); + expect(normalizeDomain("trailing@")).toBe(""); + }); +}); + +describe("classifyStratum", () => { + const policy = DEFAULT_STRATIFICATION_POLICY; + + it("classifies gmail correctly", () => { + expect(classifyStratum("user@gmail.com", policy)).toBe("gmail"); + expect(classifyStratum("user@googlemail.com", policy)).toBe("gmail"); + }); + + it("classifies naver correctly", () => { + expect(classifyStratum("user@naver.com", policy)).toBe("naver"); + }); + + it("returns other for unknown domain", () => { + expect(classifyStratum("user@example.com", policy)).toBe("other"); + }); + + it("returns unknown for malformed email", () => { + expect(classifyStratum("no-email", policy)).toBe("unknown"); + }); + + it("handles case-insensitive domains", () => { + expect(classifyStratum("User@GMAIL.COM", policy)).toBe("gmail"); + }); +}); + +describe("computeStratifiedQuotas", () => { + it("preserves row sums", () => { + const result = computeStratifiedQuotas({ + stratumSizes: { gmail: 600, naver: 300, other: 100 }, + groupExactCounts: { "variant:A": 500, "variant:B": 500 }, + groupOrder: ["variant:A", "variant:B"], + totalAudience: 1000, + }); + for (const [stratumKey, row] of Object.entries(result.quotas)) { + const rowSum = Object.values(row).reduce((s, n) => s + n, 0); + expect(rowSum).toBe( + result.stratumSizes[stratumKey as keyof typeof result.stratumSizes], + ); + } + }); + + it("preserves column sums", () => { + const result = computeStratifiedQuotas({ + stratumSizes: { gmail: 600, naver: 300, other: 100 }, + groupExactCounts: { "variant:A": 500, "variant:B": 500 }, + groupOrder: ["variant:A", "variant:B"], + totalAudience: 1000, + }); + for (const groupKey of ["variant:A", "variant:B"]) { + const colSum = Object.values(result.quotas).reduce( + (s, row) => s + (row[groupKey] ?? 0), + 0, + ); + expect(colSum).toBe(groupKey === "variant:A" ? 500 : 500); + } + }); + + it("handles 3 groups (A/B/C)", () => { + const result = computeStratifiedQuotas({ + stratumSizes: { gmail: 400, other: 200 }, + groupExactCounts: { + "variant:A": 200, + "variant:B": 200, + holdout: 200, + }, + groupOrder: ["variant:A", "variant:B", "holdout"], + totalAudience: 600, + }); + // Verify column sums + for (const gk of ["variant:A", "variant:B", "holdout"]) { + const colSum = Object.values(result.quotas).reduce( + (s, row) => s + (row[gk] ?? 0), + 0, + ); + expect(colSum).toBe(200); + } + // Verify row sums + for (const [sk, row] of Object.entries(result.quotas)) { + const rowSum = Object.values(row).reduce((s, n) => s + n, 0); + expect(rowSum).toBe(sk === "gmail" ? 400 : 200); + } + }); + + it("throws on sum mismatch", () => { + expect(() => + computeStratifiedQuotas({ + stratumSizes: { gmail: 600, other: 500 }, + groupExactCounts: { "variant:A": 500, "variant:B": 500 }, + groupOrder: ["variant:A", "variant:B"], + totalAudience: 1000, + }), + ).toThrow("Stratified quota invariant"); + }); + + it("each cell is floor or ceil of ideal", () => { + const result = computeStratifiedQuotas({ + stratumSizes: { gmail: 333, naver: 333, other: 334 }, + groupExactCounts: { "variant:A": 500, "variant:B": 500 }, + groupOrder: ["variant:A", "variant:B"], + totalAudience: 1000, + }); + for (const cell of result.cells) { + const floorIdeal = Math.floor(cell.ideal); + const ceilIdeal = Math.ceil(cell.ideal); + expect(cell.quota).toBeGreaterThanOrEqual(floorIdeal); + expect(cell.quota).toBeLessThanOrEqual(ceilIdeal); + } + }); +}); diff --git a/scripts/check-graph-architecture.ts b/scripts/check-graph-architecture.ts index 3622468b..1826d978 100644 --- a/scripts/check-graph-architecture.ts +++ b/scripts/check-graph-architecture.ts @@ -423,6 +423,13 @@ const abTestTestContracts: readonly CallPathContract[] = [ "packages/abtest/src/hypothesis.ts#computeHypothesisChecksum:function", ], }, + { + label: "Stratification tests anchor the quota matrix solver", + path: [ + "packages/abtest/tests/stratification.test.ts#packages/abtest/tests/stratification.test.ts:module", + "packages/abtest/src/stratification.ts#computeStratifiedQuotas:function", + ], + }, { label: "Store adapter tests anchor the InMemory store", path: [ From 39dafac362ff9b9dc84998e04f25e511b890128b Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 12:04:43 +0900 Subject: [PATCH 03/20] fix(abtest): validate totalAudience and assert quota convergence OpenCodeReview findings on the stratification commit: - high: totalAudience was used as the proportional divisor but never validated against the strata/groups sums. A stale or zero value produced silently skewed (or NaN) ideals. Add an explicit equality guard with a descriptive message. - medium: the Phase 2 paired-swap loop could exit early without resolving every column deficit. Add a post-loop assertion that every residual deficit is zero so an unconverged matrix fails loudly. - low: cellDeviation linearly scanned the cells array inside a nested loop. Precompute an idealLookup map and read ideals from it. --- packages/abtest/src/stratification.ts | 30 ++++++++++++++++++-- packages/abtest/tests/stratification.test.ts | 27 ++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/packages/abtest/src/stratification.ts b/packages/abtest/src/stratification.ts index 8966af69..9473e809 100644 --- a/packages/abtest/src/stratification.ts +++ b/packages/abtest/src/stratification.ts @@ -130,6 +130,13 @@ export function computeStratifiedQuotas(params: { `Stratified quota invariant: strata sum ${totalFromStrata} != groups sum ${totalFromGroups}`, ); } + if (totalAudience !== totalFromStrata) { + // totalAudience is the divisor for proportional ideals; a mismatch + // (including zero) silently skews proportions or yields NaN cells. + throw new Error( + `Stratified quota invariant: totalAudience ${totalAudience} != strata sum ${totalFromStrata}`, + ); + } const quotas: Record> = {}; const cells: StratumQuotaCell[] = []; @@ -193,12 +200,17 @@ export function computeStratifiedQuotas(params: { columnDeficit[groupKey] = exactCount - currentSum; } + // Build a lookup map of ideal values to avoid repeated linear scans of + // the cells array inside the correction loop. + const idealLookup = new Map(); + for (const cell of cells) { + idealLookup.set(`${cell.stratumKey}:${cell.groupKey}`, cell.ideal); + } + const cellDeviation = (stratumKey: string, groupKey: string): number => { const row = quotas[stratumKey]; const quota = row?.[groupKey] ?? 0; - const ideal = - cells.find((c) => c.stratumKey === stratumKey && c.groupKey === groupKey) - ?.ideal ?? 0; + const ideal = idealLookup.get(`${stratumKey}:${groupKey}`) ?? 0; return quota - ideal; }; @@ -246,6 +258,18 @@ export function computeStratifiedQuotas(params: { columnDeficit[surplusGroup] = (columnDeficit[surplusGroup] ?? 0) + 1; } + // Verify convergence: every column deficit should be resolved to zero. + // If the loop exited early (no donating row or iteration cap reached), + // the column sums would silently disagree with the exact counts. + for (const groupKey of groupOrder) { + const residual = columnDeficit[groupKey] ?? 0; + if (residual !== 0) { + throw new Error( + `Stratified quota did not converge: column "${groupKey}" has residual deficit ${residual}`, + ); + } + } + // Update cell quotas after correction. for (const cell of cells) { const row = quotas[cell.stratumKey]; diff --git a/packages/abtest/tests/stratification.test.ts b/packages/abtest/tests/stratification.test.ts index 6668b51b..850c0984 100644 --- a/packages/abtest/tests/stratification.test.ts +++ b/packages/abtest/tests/stratification.test.ts @@ -119,6 +119,33 @@ describe("computeStratifiedQuotas", () => { ).toThrow("Stratified quota invariant"); }); + it("throws when totalAudience disagrees with strata sum", () => { + expect(() => + computeStratifiedQuotas({ + stratumSizes: { gmail: 500, other: 500 }, + groupExactCounts: { "variant:A": 500, "variant:B": 500 }, + groupOrder: ["variant:A", "variant:B"], + totalAudience: 1000, + }), + ).not.toThrow(); + expect(() => + computeStratifiedQuotas({ + stratumSizes: { gmail: 500, other: 500 }, + groupExactCounts: { "variant:A": 500, "variant:B": 500 }, + groupOrder: ["variant:A", "variant:B"], + totalAudience: 1100, + }), + ).toThrow("totalAudience 1100 != strata sum 1000"); + expect(() => + computeStratifiedQuotas({ + stratumSizes: { gmail: 500, other: 500 }, + groupExactCounts: { "variant:A": 500, "variant:B": 500 }, + groupOrder: ["variant:A", "variant:B"], + totalAudience: 0, + }), + ).toThrow("totalAudience 0 != strata sum 1000"); + }); + it("each cell is floor or ceil of ideal", () => { const result = computeStratifiedQuotas({ stratumSizes: { gmail: 333, naver: 333, other: 334 }, From d0a34c37998456ca2ee75b5fb736d0dfc6f22863 Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 12:12:08 +0900 Subject: [PATCH 04/20] fix(abtest): harden hypothesis pre-registration integrity Addresses @codex and CodeRabbit review findings on Change Set A. P1: - computeHypothesisChecksum now recursively canonicalizes nested fields. The previous flat Object.keys().sort() array replacer was applied recursively by JSON.stringify, which dropped nested keys and serialized primaryMetric/expectedLift/owner/experimentScope as "{}". Tampering with any nested field after locking no longer passes verification. - validateHypothesisMetadata in strict mode now requires createdAt as a valid ISO 8601 timestamp, validates primaryMetric.type/direction against their enums, and validates expectedLift.kind plus the absolute-lift unit. P2: - Add the hypothesis shape to the shared abTest operation output schema so CLI/MCP callers can retrieve persisted pre-registration metadata instead of having Zod strip it. - Extend isStoredAbTest with isStoredHypothesis so loadStoredAbTests rejects malformed nested hypothesis records and locked-without-checksum states before hydration. - lockHypothesis validates the supplied lockedAt override as ISO 8601, rejecting empty or malformed timestamps that would produce an unverifiable lock. - Couple absolute-lift units to the primary metric: revenue_per_recipient requires currency_per_recipient, and click/conversion_rate require percentage_point. Relative lift stays unit-agnostic. - Tighten experimentFamilyKey validation to require non-empty alphanumeric segments joined by single [._-] separators, rejecting ".", "foo.", "foo..bar", and delimiter-only keys. Tests cover nested-field checksum tampering, every new strict-mode guard, the metric/unit pairing matrix, the family-key segment rules, the lockedAt override validation, and malformed persisted hypothesis rejection. --- packages/abtest/src/hypothesis.ts | 129 +++++++++++-- packages/abtest/src/operations.ts | 44 ++++- packages/abtest/src/persistence.ts | 131 ++++++++++++- packages/abtest/tests/hypothesis.test.ts | 217 ++++++++++++++++++++++ packages/abtest/tests/persistence.test.ts | 78 ++++++++ 5 files changed, 586 insertions(+), 13 deletions(-) diff --git a/packages/abtest/src/hypothesis.ts b/packages/abtest/src/hypothesis.ts index 9006a462..f8cfcf14 100644 --- a/packages/abtest/src/hypothesis.ts +++ b/packages/abtest/src/hypothesis.ts @@ -90,6 +90,7 @@ export function validateHypothesisMetadata( require("expectedLift", metadata.expectedLift); require("owner", metadata.owner); require("experimentScope", metadata.experimentScope); + require("createdAt", metadata.createdAt); if (metadata.objective !== undefined) { if (typeof metadata.objective !== "string" || metadata.objective.trim().length === 0) { @@ -105,12 +106,81 @@ export function validateHypothesisMetadata( ); } } + if (metadata.createdAt !== undefined) { + if ( + typeof metadata.createdAt !== "string" || + Number.isNaN(Date.parse(metadata.createdAt)) + ) { + throw new HypothesisValidationError( + `createdAt must be a valid ISO 8601 timestamp, received ${JSON.stringify(metadata.createdAt)}`, + ); + } + } + if (metadata.primaryMetric !== undefined) { + const pm = metadata.primaryMetric; + const validTypes = [ + "click_rate", + "conversion_rate", + "revenue_per_recipient", + ]; + if (!validTypes.includes(pm.type)) { + throw new HypothesisValidationError( + `primaryMetric.type must be one of ${validTypes.join(", ")}, received ${JSON.stringify(pm.type)}`, + ); + } + if (pm.direction !== "maximize" && pm.direction !== "minimize") { + throw new HypothesisValidationError( + `primaryMetric.direction must be "maximize" or "minimize", received ${JSON.stringify(pm.direction)}`, + ); + } + } if (metadata.expectedLift !== undefined) { - if (!Number.isFinite( - metadata.expectedLift.value, - ) || metadata.expectedLift.value <= 0) { + const lift = metadata.expectedLift; + const rawKind = (lift as { kind?: unknown }).kind; + if (rawKind !== "relative" && rawKind !== "absolute") { + throw new HypothesisValidationError( + `expectedLift.kind must be "relative" or "absolute", received ${JSON.stringify(rawKind)}`, + ); + } + if (!Number.isFinite(lift.value) || lift.value <= 0) { throw new HypothesisValidationError( - `expectedLift.value must be finite and positive, received ${metadata.expectedLift.value}`, + `expectedLift.value must be finite and positive, received ${lift.value}`, + ); + } + if (lift.kind === "absolute") { + const validUnits = ["percentage_point", "currency_per_recipient"]; + if (!validUnits.includes(lift.unit)) { + throw new HypothesisValidationError( + `expectedLift.unit must be one of ${validUnits.join(", ")} for absolute lift, received ${JSON.stringify(lift.unit)}`, + ); + } + } + } + // Couple absolute-lift units to the primary metric so the pre-registered + // lift has an interpretable meaning. A click_rate / conversion_rate metric + // may use percentage_point lift; a revenue_per_recipient metric must use + // currency_per_recipient. Relative lift is unit-agnostic. + if ( + metadata.primaryMetric !== undefined && + metadata.expectedLift !== undefined && + metadata.expectedLift.kind === "absolute" + ) { + const metricType = metadata.primaryMetric.type; + const unit = metadata.expectedLift.unit; + if ( + metricType === "revenue_per_recipient" && + unit !== "currency_per_recipient" + ) { + throw new HypothesisValidationError( + `revenue_per_recipient metric requires currency_per_recipient absolute lift, received ${JSON.stringify(unit)}`, + ); + } + if ( + (metricType === "click_rate" || metricType === "conversion_rate") && + unit !== "percentage_point" + ) { + throw new HypothesisValidationError( + `${metricType} metric requires percentage_point absolute lift, received ${JSON.stringify(unit)}`, ); } } @@ -133,9 +203,12 @@ export function validateHypothesisMetadata( "experimentScope.experimentFamilyKey must be a non-empty string", ); } - if (!scope.experimentFamilyKey.match(/^[a-z0-9._-]+$/)) { + // Reject delimiter-only and empty-segment keys (".", "foo.", "foo..bar") + // by requiring one or more alphanumeric segments joined by single + // separators from [._-]. + if (!scope.experimentFamilyKey.match(/^[a-z0-9]+(?:[._-][a-z0-9]+)*$/)) { throw new HypothesisValidationError( - `experimentFamilyKey must match [a-z0-9._-]+, received "${scope.experimentFamilyKey}"`, + `experimentFamilyKey must be dotted alphanumeric segments (e.g. "onboarding.activation"), received "${scope.experimentFamilyKey}"`, ); } if ( @@ -157,15 +230,43 @@ export function validateHypothesisMetadata( } } +/** + * Recursively canonicalize a value for stable serialization: + * - Plain objects have their keys sorted alphabetically. + * - Arrays are preserved in order with each element canonicalized. + * - Primitives are returned as-is. + * This guarantees that two objects with the same content (in any key + * order) produce identical JSON, which a flat `Object.keys(...).sort()` + * array replacer cannot do because JSON.stringify applies the replacer + * recursively and would drop nested keys not present in the top-level + * allowlist. + */ +function canonicalize(value: unknown): unknown { + if (Array.isArray(value)) { + return value.map(canonicalize); + } + if (value !== null && typeof value === "object") { + const obj = value as Record; + const sorted: Record = {}; + for (const key of Object.keys(obj).sort()) { + sorted[key] = canonicalize(obj[key]); + } + return sorted; + } + return value; +} + /** * Compute a canonical SHA-256 checksum for a HypothesisMetadata object. * The checksum excludes `lockedAt` and `checksum` themselves so the - * same content always produces the same hash. + * same content always produces the same hash. Nested fields + * (primaryMetric, expectedLift, owner, experimentScope) are recursively + * canonicalized so any change to them invalidates the checksum. */ export function computeHypothesisChecksum( metadata: HypothesisMetadata, ): string { - const canonical = { + const canonical = canonicalize({ objective: metadata.objective, hypothesis: metadata.hypothesis, primaryMetric: metadata.primaryMetric, @@ -173,15 +274,16 @@ export function computeHypothesisChecksum( owner: { id: metadata.owner.id, displayName: metadata.owner.displayName }, experimentScope: metadata.experimentScope, createdAt: metadata.createdAt, - }; - const json = JSON.stringify(canonical, Object.keys(canonical).sort()); + }) as Record; + const json = JSON.stringify(canonical); return createHash("sha256").update(json, "utf8").digest("hex"); } /** * Lock a hypothesis by computing its checksum and setting lockedAt. * Returns a new object with checksum and lockedAt populated. - * Throws if the hypothesis is already locked. + * Throws if the hypothesis is already locked or if the supplied lock + * timestamp override is not a valid ISO 8601 string. */ export function lockHypothesis( metadata: HypothesisMetadata, @@ -192,6 +294,11 @@ export function lockHypothesis( "Hypothesis is already locked; create a new test revision to change it", ); } + if (typeof lockedAt !== "string" || Number.isNaN(Date.parse(lockedAt))) { + throw new HypothesisValidationError( + `lockedAt override must be a valid ISO 8601 timestamp, received ${JSON.stringify(lockedAt)}`, + ); + } validateHypothesisMetadata(metadata, true); const checksum = computeHypothesisChecksum(metadata); return { ...metadata, lockedAt, checksum }; diff --git a/packages/abtest/src/operations.ts b/packages/abtest/src/operations.ts index 7ac3afaa..bf6e8e2e 100644 --- a/packages/abtest/src/operations.ts +++ b/packages/abtest/src/operations.ts @@ -160,7 +160,49 @@ const abTestSchema = z.object({ assignmentProvenance: z .enum(["manifest_v1", "legacy_unavailable"]) .optional(), - }); + // Pre-registration hypothesis metadata (advanced experimentation Change + // Set A). Optional so existing records without a hypothesis still parse. + // Kept structurally aligned with HypothesisMetadata so CLI/MCP callers can + // retrieve the pre-registration through the shared operation output. + hypothesis: z + .object({ + objective: z.string(), + hypothesis: z.string(), + primaryMetric: z.object({ + type: z.enum([ + "click_rate", + "conversion_rate", + "revenue_per_recipient", + ]), + direction: z.enum(["maximize", "minimize"]), + }), + expectedLift: z.union([ + z.object({ + kind: z.literal("relative"), + value: z.number().finite().positive(), + }), + z.object({ + kind: z.literal("absolute"), + value: z.number().finite().positive(), + unit: z.enum(["percentage_point", "currency_per_recipient"]), + }), + ]), + owner: z.object({ + id: z.string(), + displayName: z.string().optional(), + }), + experimentScope: z.object({ + channel: z.literal("email"), + experimentFamilyKey: z.string(), + attributionWindowHours: z.number().finite().positive(), + exclusionWindowHours: z.number().finite().nonnegative(), + }), + createdAt: z.string(), + lockedAt: z.string().optional(), + checksum: z.string().optional(), + }) + .optional(), +}); const testResultsSchema = z.object({ variantId: z.string(), diff --git a/packages/abtest/src/persistence.ts b/packages/abtest/src/persistence.ts index dabc8ae6..317a8454 100644 --- a/packages/abtest/src/persistence.ts +++ b/packages/abtest/src/persistence.ts @@ -238,10 +238,139 @@ function isStoredAbTest(value: unknown): boolean { isPositiveInteger(value.minimumTestSampleSize)) && (value.assignmentProvenance === undefined || value.assignmentProvenance === "manifest_v1" || - value.assignmentProvenance === "legacy_unavailable") + value.assignmentProvenance === "legacy_unavailable") && + // Pre-registration hypothesis: optional, but the nested shape and the + // locked-state checksum invariant are validated when present so that + // loadStoredAbTests never hydrates malformed or tampered metadata. + (value.hypothesis === undefined || isStoredHypothesis(value.hypothesis)) ); } +const HYPOTHESIS_METRIC_TYPES = new Set([ + "click_rate", + "conversion_rate", + "revenue_per_recipient", +]); +const HYPOTHESIS_DIRECTIONS = new Set(["maximize", "minimize"]); +const HYPOTHESIS_ABSOLUTE_UNITS = new Set([ + "percentage_point", + "currency_per_recipient", +]); + +/** + * Validate a persisted hypothesis record. Mirrors the runtime validation in + * hypothesis.ts but as a structural guard so loadStoredAbTests rejects + * malformed or tampered metadata before it reaches launch/report code. + * When lockedAt is present, checksum must also be present and match the + * recomputed canonical checksum. + */ +function isStoredHypothesis(value: unknown): boolean { + if (!isRecord(value)) { + return false; + } + if (typeof value.objective !== "string" || value.objective.trim() === "") { + return false; + } + if (typeof value.hypothesis !== "string" || value.hypothesis.trim() === "") { + return false; + } + if (typeof value.createdAt !== "string" || !isValidTimestamp( + value.createdAt, + )) { + return false; + } + // primaryMetric + const pm = value.primaryMetric; + if ( + !isRecord(pm) || + typeof pm.type !== "string" || + !HYPOTHESIS_METRIC_TYPES.has(pm.type) || + typeof pm.direction !== "string" || + !HYPOTHESIS_DIRECTIONS.has(pm.direction) + ) { + return false; + } + // expectedLift discriminated union + const lift = value.expectedLift; + if (!isRecord(lift)) { + return false; + } + if (lift.kind === "relative") { + if ( + typeof lift.value !== "number" || + !Number.isFinite(lift.value) || + lift.value <= 0 + ) { + return false; + } + } else if (lift.kind === "absolute") { + if ( + typeof lift.value !== "number" || + !Number.isFinite(lift.value) || + lift.value <= 0 || + typeof lift.unit !== "string" || + !HYPOTHESIS_ABSOLUTE_UNITS.has(lift.unit) + ) { + return false; + } + } else { + return false; + } + // metric/unit coupling for absolute lifts + if ( + lift.kind === "absolute" && + typeof lift.unit === "string" && + typeof pm.type === "string" + ) { + if (pm.type === "revenue_per_recipient" && lift.unit !== "currency_per_recipient") { + return false; + } + if ( + (pm.type === "click_rate" || pm.type === "conversion_rate") && + lift.unit !== "percentage_point" + ) { + return false; + } + } + // owner + const owner = value.owner; + if ( + !isRecord(owner) || + typeof owner.id !== "string" || + owner.id.trim() === "" || + (owner.displayName !== undefined && typeof owner.displayName !== "string") + ) { + return false; + } + // experimentScope + const scope = value.experimentScope; + if ( + !isRecord(scope) || + scope.channel !== "email" || + typeof scope.experimentFamilyKey !== "string" || + typeof scope.attributionWindowHours !== "number" || + !Number.isFinite(scope.attributionWindowHours) || + scope.attributionWindowHours <= 0 || + typeof scope.exclusionWindowHours !== "number" || + !Number.isFinite(scope.exclusionWindowHours) || + scope.exclusionWindowHours < 0 + ) { + return false; + } + // locked-state invariant: if lockedAt is present, checksum must be too. + if (value.lockedAt !== undefined) { + if (typeof value.lockedAt !== "string" || !isValidTimestamp( + value.lockedAt, + )) { + return false; + } + if (typeof value.checksum !== "string" || value.checksum.length !== 64) { + return false; + } + } + return true; +} + function isNonNegativeInteger(value: unknown): value is number { return typeof value === "number" && Number.isInteger(value) && value >= 0; } diff --git a/packages/abtest/tests/hypothesis.test.ts b/packages/abtest/tests/hypothesis.test.ts index daffe219..7ff970ea 100644 --- a/packages/abtest/tests/hypothesis.test.ts +++ b/packages/abtest/tests/hypothesis.test.ts @@ -124,6 +124,159 @@ describe("validateHypothesisMetadata", () => { ), ).toThrow(HypothesisValidationError); }); + + it("requires createdAt in strict mode", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ createdAt: undefined }), + true, + ), + ).toThrow(HypothesisValidationError); + }); + + it("rejects malformed createdAt", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ createdAt: "not-a-date" }), + ), + ).toThrow(HypothesisValidationError); + }); + + it("rejects invalid primaryMetric.type in strict mode", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + primaryMetric: { + type: "bogus" as "click_rate", + direction: "maximize", + }, + }), + true, + ), + ).toThrow(HypothesisValidationError); + }); + + it("rejects invalid primaryMetric.direction", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + primaryMetric: { + type: "click_rate", + direction: "sideways" as "maximize", + }, + }), + ), + ).toThrow(HypothesisValidationError); + }); + + it("rejects invalid expectedLift.kind", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + expectedLift: { + kind: "bogus" as "relative", + value: 0.1, + } as HypothesisMetadata["expectedLift"], + }), + ), + ).toThrow(HypothesisValidationError); + }); + + it("rejects absolute lift without a valid unit", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + expectedLift: { + kind: "absolute", + value: 1, + unit: "bogus" as "percentage_point", + }, + }), + ), + ).toThrow(HypothesisValidationError); + }); + + it("rejects incompatible revenue metric with percentage_point lift", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + primaryMetric: { + type: "revenue_per_recipient", + direction: "maximize", + }, + expectedLift: { + kind: "absolute", + value: 1, + unit: "percentage_point", + }, + }), + ), + ).toThrow(HypothesisValidationError); + }); + + it("rejects click metric with currency_per_recipient lift", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + expectedLift: { + kind: "absolute", + value: 1, + unit: "currency_per_recipient", + }, + }), + ), + ).toThrow(HypothesisValidationError); + }); + + it("accepts compatible revenue metric with currency_per_recipient lift", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + primaryMetric: { + type: "revenue_per_recipient", + direction: "maximize", + }, + expectedLift: { + kind: "absolute", + value: 1, + unit: "currency_per_recipient", + }, + }), + ), + ).not.toThrow(); + }); + + it("rejects delimiter-only experimentFamilyKey", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + experimentScope: { + channel: "email", + experimentFamilyKey: ".", + attributionWindowHours: 72, + exclusionWindowHours: 168, + }, + }), + ), + ).toThrow(HypothesisValidationError); + }); + + it("rejects empty family-key segments", () => { + for (const bad of ["foo.", ".foo", "foo..bar", "-"]) { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + experimentScope: { + channel: "email", + experimentFamilyKey: bad, + attributionWindowHours: 72, + exclusionWindowHours: 168, + }, + }), + ), + ).toThrow(HypothesisValidationError); + } + }); }); describe("computeHypothesisChecksum", () => { @@ -153,6 +306,49 @@ describe("computeHypothesisChecksum", () => { computeHypothesisChecksum(base), ); }); + + it("changes when a nested primaryMetric field changes", () => { + const base = computeHypothesisChecksum(makeHypothesis()); + const changed = computeHypothesisChecksum( + makeHypothesis({ + primaryMetric: { type: "conversion_rate", direction: "maximize" }, + }), + ); + expect(base).not.toBe(changed); + }); + + it("changes when a nested expectedLift field changes", () => { + const base = computeHypothesisChecksum(makeHypothesis()); + const changed = computeHypothesisChecksum( + makeHypothesis({ + expectedLift: { kind: "relative", value: 0.2 }, + }), + ); + expect(base).not.toBe(changed); + }); + + it("changes when a nested owner field changes", () => { + const base = computeHypothesisChecksum(makeHypothesis()); + const changed = computeHypothesisChecksum( + makeHypothesis({ owner: { id: "user-2" } }), + ); + expect(base).not.toBe(changed); + }); + + it("changes when a nested experimentScope field changes", () => { + const base = computeHypothesisChecksum(makeHypothesis()); + const changed = computeHypothesisChecksum( + makeHypothesis({ + experimentScope: { + channel: "email", + experimentFamilyKey: "onboarding.welcome.subject", + attributionWindowHours: 48, + exclusionWindowHours: 168, + }, + }), + ); + expect(base).not.toBe(changed); + }); }); describe("lockHypothesis", () => { @@ -176,6 +372,27 @@ describe("lockHypothesis", () => { ), ).toThrow(HypothesisValidationError); }); + + it("rejects an empty lockedAt override", () => { + expect(() => lockHypothesis(makeHypothesis(), "")).toThrow( + HypothesisValidationError, + ); + }); + + it("rejects a malformed lockedAt override", () => { + expect(() => lockHypothesis(makeHypothesis(), "not-a-date")).toThrow( + HypothesisValidationError, + ); + }); + + it("accepts a valid lockedAt override", () => { + const locked = lockHypothesis( + makeHypothesis(), + "2026-07-24T01:00:00Z", + ); + expect(locked.lockedAt).toBe("2026-07-24T01:00:00Z"); + expect(verifyHypothesisChecksum(locked)).toBe(true); + }); }); describe("verifyHypothesisChecksum", () => { diff --git a/packages/abtest/tests/persistence.test.ts b/packages/abtest/tests/persistence.test.ts index e054eeae..f92c54eb 100644 --- a/packages/abtest/tests/persistence.test.ts +++ b/packages/abtest/tests/persistence.test.ts @@ -241,4 +241,82 @@ describe("A/B test persistence", () => { ); } }); + + test("rejects malformed persisted hypothesis metadata", async () => { + const storePath = await createStorePath(); + const validTest = createTest("one"); + const validHypothesis = { + objective: "Increase CTR", + hypothesis: "Shorter subject lifts CTR", + primaryMetric: { + type: "click_rate", + direction: "maximize", + }, + expectedLift: { kind: "relative", value: 0.1 }, + owner: { id: "user-1" }, + experimentScope: { + channel: "email", + experimentFamilyKey: "onboarding.welcome", + attributionWindowHours: 72, + exclusionWindowHours: 168, + }, + createdAt: "2026-07-24T00:00:00.000Z", + }; + // A valid hypothesis round-trips through the store. + await saveStoredAbTests([validTest], storePath); + await writeFile( + storePath, + `${JSON.stringify({ + version: 1, + tests: [{ ...validTest, hypothesis: validHypothesis }], + })}\n`, + "utf8", + ); + await expect(loadStoredAbTests(storePath)).resolves.toHaveLength(1); + + // A hypothesis with a bogus metric type is rejected. + await writeFile( + storePath, + `${JSON.stringify({ + version: 1, + tests: [ + { + ...validTest, + hypothesis: { + ...validHypothesis, + primaryMetric: { + type: "bogus", + direction: "maximize", + }, + }, + }, + ], + })}\n`, + "utf8", + ); + await expect(loadStoredAbTests(storePath)).rejects.toThrow( + "test 0 failed schema validation", + ); + + // A hypothesis locked without a checksum is rejected. + await writeFile( + storePath, + `${JSON.stringify({ + version: 1, + tests: [ + { + ...validTest, + hypothesis: { + ...validHypothesis, + lockedAt: "2026-07-24T01:00:00.000Z", + }, + }, + ], + })}\n`, + "utf8", + ); + await expect(loadStoredAbTests(storePath)).rejects.toThrow( + "test 0 failed schema validation", + ); + }); }); From 60ef21407516b62bc2d7bf4bd60b6c53728d9947 Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 12:18:37 +0900 Subject: [PATCH 05/20] fix(abtest): verify locked hypothesis checksum at load time OpenCodeReview findings on the hardening commit: - medium: isStoredHypothesis only checked that checksum was a 64-char string, but a tampered locked record (valid format, wrong hash) would still hydrate. Now re-verifies the checksum cryptographically via verifyHypothesisChecksum so post-lock tampering is rejected at load. - medium: isStoredHypothesis validated experimentFamilyKey only as a string, weaker than the runtime segment rules. Mirror the /^[a-z0-9]+(?:[._-][a-z0-9]+)*$/ regex so load-time validation matches creation-time validation. - low: lockHypothesis validated the lockedAt override before the metadata, surfacing the less relevant timestamp error first. Reorder to validate metadata first, then the timestamp override. Skipped the 'remove the rawKind cast' suggestion: the discriminated union narrows an invalid kind to never, so the runtime guard cannot compile without the cast. Kept the cast to preserve the defensive runtime check. Tests cover a properly locked hypothesis round-trip, post-lock tampering rejection, and malformed family-key rejection at load time. --- packages/abtest/src/hypothesis.ts | 4 +- packages/abtest/src/persistence.ts | 24 +++++++-- packages/abtest/tests/persistence.test.ts | 59 +++++++++++++++++++++++ 3 files changed, 81 insertions(+), 6 deletions(-) diff --git a/packages/abtest/src/hypothesis.ts b/packages/abtest/src/hypothesis.ts index f8cfcf14..d570b4ef 100644 --- a/packages/abtest/src/hypothesis.ts +++ b/packages/abtest/src/hypothesis.ts @@ -294,12 +294,14 @@ export function lockHypothesis( "Hypothesis is already locked; create a new test revision to change it", ); } + // Validate the primary metadata first so domain errors surface before + // the timestamp override check; the lockedAt override is secondary input. + validateHypothesisMetadata(metadata, true); if (typeof lockedAt !== "string" || Number.isNaN(Date.parse(lockedAt))) { throw new HypothesisValidationError( `lockedAt override must be a valid ISO 8601 timestamp, received ${JSON.stringify(lockedAt)}`, ); } - validateHypothesisMetadata(metadata, true); const checksum = computeHypothesisChecksum(metadata); return { ...metadata, lockedAt, checksum }; } diff --git a/packages/abtest/src/persistence.ts b/packages/abtest/src/persistence.ts index 317a8454..ed5e22ae 100644 --- a/packages/abtest/src/persistence.ts +++ b/packages/abtest/src/persistence.ts @@ -11,6 +11,7 @@ import { import type { ListmonkClient } from "@listmonk-ops/openapi"; import { AbTestNotFoundError } from "./errors"; import { createAbTestExecutors, type AbTestExecutors } from "./factory"; +import { verifyHypothesisChecksum } from "./hypothesis"; import type { AbTest } from "./types"; export { AbTestNotFoundError } from "./errors"; @@ -348,6 +349,9 @@ function isStoredHypothesis(value: unknown): boolean { !isRecord(scope) || scope.channel !== "email" || typeof scope.experimentFamilyKey !== "string" || + // Mirror the runtime segment rules so load-time validation is at least + // as strict as creation-time validation. + !/^[a-z0-9]+(?:[._-][a-z0-9]+)*$/.test(scope.experimentFamilyKey) || typeof scope.attributionWindowHours !== "number" || !Number.isFinite(scope.attributionWindowHours) || scope.attributionWindowHours <= 0 || @@ -357,14 +361,24 @@ function isStoredHypothesis(value: unknown): boolean { ) { return false; } - // locked-state invariant: if lockedAt is present, checksum must be too. + // locked-state invariant: when lockedAt is present the checksum must be a + // 64-character hex string AND must cryptographically match the recomputed + // canonical checksum, so tampered records are rejected at load time. if (value.lockedAt !== undefined) { - if (typeof value.lockedAt !== "string" || !isValidTimestamp( - value.lockedAt, - )) { + if ( + typeof value.lockedAt !== "string" || !isValidTimestamp(value.lockedAt) + ) { return false; } - if (typeof value.checksum !== "string" || value.checksum.length !== 64) { + if ( + typeof value.checksum !== "string" || + value.checksum.length !== 64 || + // The record has been structurally validated above; verify the + // checksum cryptographically rejects tampered locked hypotheses. + !verifyHypothesisChecksum( + value as unknown as Parameters[0], + ) + ) { return false; } } diff --git a/packages/abtest/tests/persistence.test.ts b/packages/abtest/tests/persistence.test.ts index f92c54eb..75ff9044 100644 --- a/packages/abtest/tests/persistence.test.ts +++ b/packages/abtest/tests/persistence.test.ts @@ -11,6 +11,7 @@ import { saveStoredAbTests, withStoredAbTestExecutors, } from "../src/persistence"; +import { lockHypothesis } from "../src/hypothesis"; import type { AbTest } from "../src/types"; const temporaryDirectories: string[] = []; @@ -318,5 +319,63 @@ describe("A/B test persistence", () => { await expect(loadStoredAbTests(storePath)).rejects.toThrow( "test 0 failed schema validation", ); + + // A properly locked hypothesis round-trips through the store. + const locked = lockHypothesis( + validHypothesis as Parameters[0], + "2026-07-24T01:00:00.000Z", + ); + await writeFile( + storePath, + `${JSON.stringify({ + version: 1, + tests: [{ ...validTest, hypothesis: locked }], + })}\n`, + "utf8", + ); + await expect(loadStoredAbTests(storePath)).resolves.toHaveLength(1); + + // A locked hypothesis whose content was tampered after locking + // (checksum no longer matches) is rejected at load time. + const tamperedLocked = { + ...locked, + objective: "Tampered objective", + }; + await writeFile( + storePath, + `${JSON.stringify({ + version: 1, + tests: [{ ...validTest, hypothesis: tamperedLocked }], + })}\n`, + "utf8", + ); + await expect(loadStoredAbTests(storePath)).rejects.toThrow( + "test 0 failed schema validation", + ); + + // A hypothesis with a malformed family key is rejected at load time, + // mirroring the creation-time segment rules. + await writeFile( + storePath, + `${JSON.stringify({ + version: 1, + tests: [ + { + ...validTest, + hypothesis: { + ...validHypothesis, + experimentScope: { + ...validHypothesis.experimentScope, + experimentFamilyKey: "foo..bar", + }, + }, + }, + ], + })}\n`, + "utf8", + ); + await expect(loadStoredAbTests(storePath)).rejects.toThrow( + "test 0 failed schema validation", + ); }); }); From 461952de9784b93c03bc81cee1cf155dee922f97 Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 12:47:42 +0900 Subject: [PATCH 06/20] fix(abtest): wire hypothesis+stratification into production paths Addresses the second @codex review's P1/P2 findings on PR #46. P1: - Hypothesis is now wired into creation: CreateAbTestInput/AbTestConfig gain an optional hypothesis field, basic.ts maps snake_case input to the domain type, and AbTestService.createTest locks it (if unlocked) before any provisioning so the assignment manifest is bound to a frozen checksummed hypothesis. - Persistence now requires a hypothesis to be present and locked whenever an assignmentManifest exists, enforcing the pre-registration guarantee that hypothesis content cannot change after recipient assignment. - Stratification is now called in production: segmentSubscribersForHoldout computes the recipient-domain quota matrix from the resolved audience (when a stratification policy is enabled and emails are available) and stores it on AbTest.stratification for reporting/validation. - Document the hypothesis and stratification APIs bilingually (EN/KO) in the package README, covering contracts, validation rules, and usage. P2: - Tighten createdAt/lockedAt to strict ISO 8601: reject values Date.parse silently accepts ("0", "01/02/03", overflowed "2026-02-30"). - Guard nested metadata access so null/non-object primaryMetric/expectedLift/ owner/experimentScope and non-primitive fields raise HypothesisValidationError instead of raw TypeError. - Populate assignmentProvenance during provisioning: manifest_v1 for holdout, legacy_unavailable for full-split. - Preserve floor/ceiling bounds during quota correction by preferring swaps that keep donor >= floor(ideal) and receiver <= ceil(ideal), with a fallback to minimize deviation when column totals otherwise forbid a bounded swap. - Normalize configured provider domains before classification so mixed-case or trailing-dot entries match subscriber domains correctly. Verified against 8000 randomized stratification trials and 240 abtest tests; full bun run check (198 architecture paths) and build pass. --- packages/abtest/README.md | 145 +++++++++++++++++++ packages/abtest/src/abtest-service.ts | 18 +++ packages/abtest/src/audience.ts | 4 + packages/abtest/src/basic.ts | 35 +++++ packages/abtest/src/hypothesis.ts | 99 +++++++++++-- packages/abtest/src/listmonk-integration.ts | 55 +++++++ packages/abtest/src/operations.ts | 52 +++++++ packages/abtest/src/persistence.ts | 8 +- packages/abtest/src/stratification.ts | 71 +++++++-- packages/abtest/src/types.ts | 32 ++++ packages/abtest/tests/basic.test.ts | 44 ++++++ packages/abtest/tests/hypothesis.test.ts | 50 +++++++ packages/abtest/tests/stratification.test.ts | 42 ++++++ 13 files changed, 626 insertions(+), 29 deletions(-) diff --git a/packages/abtest/README.md b/packages/abtest/README.md index ede215b2..138bcfae 100644 --- a/packages/abtest/README.md +++ b/packages/abtest/README.md @@ -661,3 +661,148 @@ assignment and chunked bulk list membership: ## License MIT License - see LICENSE file for details. + +## Hypothesis pre-registration (advanced experimentation) + +An A/B test can carry a **pre-registered hypothesis** that is locked +(checksummed) before recipient assignment. After locking, the hypothesis +cannot change without discarding the assignment manifest and provisioning. +This prevents post-hoc hypothesis adjustment (p-hacking) and gives reports a +stable reference. + +### Locking and verification + +```typescript +import { + lockHypothesis, + verifyHypothesisChecksum, + validateHypothesisMetadata, +} from "@listmonk-ops/abtest"; + +const metadata = { + objective: "Increase click-through rate on the welcome email", + hypothesis: "A shorter subject line will increase CTR by 10%", + primaryMetric: { type: "click_rate", direction: "maximize" }, + expectedLift: { kind: "relative", value: 0.1 }, + owner: { id: "user-1", displayName: "Test User" }, + experimentScope: { + channel: "email", + experimentFamilyKey: "onboarding.welcome.subject", + attributionWindowHours: 72, + exclusionWindowHours: 168, + }, + createdAt: new Date().toISOString(), +}; + +validateHypothesisMetadata(metadata, true); // strict launch check +const locked = lockHypothesis(metadata); +verifyHypothesisChecksum(locked); // true +``` + +The checksum recursively canonicalizes nested fields +(`primaryMetric`, `expectedLift`, `owner`, `experimentScope`, `createdAt`), +so any change after locking invalidates it. + +### Wiring through creation + +Pass a `hypothesis` field to `createAbTest`. The service locks it before +provisioning, so the assignment manifest is always bound to a frozen +hypothesis: + +```typescript +const test = await abTestExecutors.createAbTest({ + name: "Subject Line Test", + variants: [/* ... */], + lists: [1, 2], + hypothesis: { + objective: "Increase CTR", + hypothesis: "Shorter subject lifts CTR", + primary_metric: { type: "click_rate", direction: "maximize" }, + expected_lift: { kind: "relative", value: 0.1 }, + owner: { id: "user-1" }, + experiment_scope: { + channel: "email", + experiment_family_key: "onboarding.welcome", + attribution_window_hours: 72, + exclusion_window_hours: 168, + }, + }, +}); +``` + +### Validation rules + +- `createdAt` and `lockedAt` must be strict ISO 8601 timestamps (the year-zero + string `"0"`, localized formats like `"01/02/03"`, and overflowed dates like + `"2026-02-30"` are rejected). +- `primaryMetric.type` ∈ `click_rate | conversion_rate | revenue_per_recipient`. +- `primaryMetric.direction` ∈ `maximize | minimize`. +- `expectedLift.kind` ∈ `relative | absolute`. Absolute lifts require a `unit` + ∈ `percentage_point | currency_per_recipient`. +- Metric/unit coupling: `revenue_per_recipient` requires + `currency_per_recipient` absolute lift; `click_rate`/`conversion_rate` + require `percentage_point`. Relative lift is unit-agnostic. +- `experimentScope.experimentFamilyKey` must be dotted alphanumeric segments + (`onboarding.activation.day1`); `.`, `foo.`, and `foo..bar` are rejected. + +### 가설 사전 등록 (Korean) + +A/B 테스트에 **사전 등록된 가설**을 설정하면 수신자 할당 전에 잠금(체크섬) +처리됩니다. 잠금 후에는 할당 매니페스트와 프로비저닝을 폐기하지 않는 한 +가설을 변경할 수 없습니다. 이는 사후 가설 조정(p-hacking)을 방지하고 +보고서에 안정적인 기준점을 제공합니다. + +- `createAbTest`에 `hypothesis` 필드를 전달하면 서비스가 프로비저닝 전에 + 잠금 처리합니다. +- 체크섬은 중첩 필드(`primaryMetric`, `expectedLift`, `owner`, + `experimentScope`, `createdAt`)를 재귀적으로 정규화하므로 잠금 후 어떤 + 변경도 무효화됩니다. +- `createdAt`/`lockedAt`은 엄격한 ISO 8601이어야 합니다. +- `experimentFamilyKey`는 점으로 구분된 영숫자 세그먼트여야 합니다. + +## Recipient-domain stratification (advanced experimentation) + +Stratification classifies subscribers by email-domain provider and computes a +**constrained quota matrix** so each provider stratum gets a proportional share +of every variant/holdout group. This prevents a single large provider (e.g. +Gmail) from dominating one variant and skewing results. + +```typescript +import { + classifyStratum, + computeStratifiedQuotas, + DEFAULT_STRATIFICATION_POLICY, +} from "@listmonk-ops/abtest"; + +const policy = { ...DEFAULT_STRATIFICATION_POLICY, enabled: true }; +const stratum = classifyStratum("user@gmail.com", policy); // "gmail" + +const result = computeStratifiedQuotas({ + stratumSizes: { gmail: 600, naver: 300, other: 100 }, + groupExactCounts: { "variant:A": 500, "variant:B": 500 }, + groupOrder: ["variant:A", "variant:B"], + totalAudience: 1000, +}); +``` + +The solver uses the largest-remainder method per stratum row, then a paired-swap +column correction that preserves row sums while matching exact group column +counts. Configured domains in the provider map are normalized with the same +rules applied to subscriber emails, so mixed-case entries like `"GMAIL.COM"` +match correctly. + +During holdout provisioning, when a stratification policy is enabled and the +resolved audience carries emails, the quota matrix is computed and stored on +the `AbTest.stratification` field for reporting and validation. + +### 수신자 도메인 층화 (Korean) + +층화는 구독자를 이메일 도메인 제공자별로 분류하고, 각 제공자 층(stratum)이 +모든 변형/홀드아웃 그룹의 비례 배분을 받도록 **제약된 할당량 행렬**을 +계산합니다. 단일 대형 제공자(예: Gmail)가 하나의 변형을 독점하여 결과를 +왜곡하는 것을 방지합니다. + +- `classifyStratum`으로 구독자를 분류하고, `computeStratifiedQuotas`로 + 할당량 행렬을 계산합니다. +- 홀드아웃 프로비저닝 시 층화 정책이 활성화되어 있으면 할당량 행렬이 + `AbTest.stratification`에 저장되어 보고/검증에 사용됩니다. diff --git a/packages/abtest/src/abtest-service.ts b/packages/abtest/src/abtest-service.ts index 9bbdbe3a..1e604e99 100644 --- a/packages/abtest/src/abtest-service.ts +++ b/packages/abtest/src/abtest-service.ts @@ -11,6 +11,7 @@ import { DEFAULT_STATISTICAL_POLICY, fixedHorizonGate, } from "./statistics"; +import { lockHypothesis } from "./hypothesis"; import type { AbTest, AbTestConfig, @@ -178,6 +179,13 @@ export class AbTestService { autoDeployWinner, campaignMappings: [], testListMappings: [], + // Lock the pre-registration hypothesis before any provisioning so the + // assignment manifest is bound to a frozen, checksummed hypothesis. + hypothesis: config.hypothesis + ? config.hypothesis.lockedAt + ? config.hypothesis + : lockHypothesis(config.hypothesis) + : undefined, }; // Create Listmonk campaigns if integration is available @@ -224,6 +232,14 @@ export class AbTestService { abTest.assignmentSeed = segmentationResult.assignmentSeed; abTest.audienceSnapshot = segmentationResult.audienceSnapshot; abTest.assignmentManifest = segmentationResult.assignmentManifest; + // Record that recipients were assigned through a deterministic + // manifest, so consumers can distinguish it from legacy splits. + abTest.assignmentProvenance = "manifest_v1"; + // Capture the stratified quota matrix when a stratification + // policy produced one, so reports can show per-provider shares. + if (segmentationResult.stratification) { + abTest.stratification = segmentationResult.stratification; + } } else { // Use full-split methodology (legacy) testListMappings = await this.listmonkIntegration.segmentSubscribers( @@ -238,6 +254,8 @@ export class AbTestService { ); testGroupSize = totalSubscribers; holdoutGroupSize = 0; + // Full-split provisioning predates deterministic manifests. + abTest.assignmentProvenance = "legacy_unavailable"; } provisionedResources = { ...provisionedResources, diff --git a/packages/abtest/src/audience.ts b/packages/abtest/src/audience.ts index efc6b5f7..b3c7856a 100644 --- a/packages/abtest/src/audience.ts +++ b/packages/abtest/src/audience.ts @@ -30,6 +30,9 @@ export interface AudienceMember { subscriberId: number; /** Stable UUID used for identity, dedupe, checksum, and deterministic assignment. */ subscriberUuid: string; + /** Subscriber email, used for recipient-domain stratification. Optional + * because legacy resolvers and test fixtures may not populate it. */ + email?: string; } export interface AudienceSnapshot { @@ -228,6 +231,7 @@ export function createListmonkAudienceResolver( collected.push({ subscriberId: numericId, subscriberUuid: uuid, + email: subscriber.email, }); } } diff --git a/packages/abtest/src/basic.ts b/packages/abtest/src/basic.ts index 5c239c44..1024efdb 100644 --- a/packages/abtest/src/basic.ts +++ b/packages/abtest/src/basic.ts @@ -48,6 +48,41 @@ export class CreateAbTestCommand { ignoreStatisticalWarnings: input.ignore_sample_size_warnings || false, durationHours: input.duration_hours, launchAt: input.launch_at, + hypothesis: input.hypothesis + ? { + objective: input.hypothesis.objective, + hypothesis: input.hypothesis.hypothesis, + primaryMetric: { + type: input.hypothesis.primary_metric.type, + direction: input.hypothesis.primary_metric.direction, + }, + expectedLift: + input.hypothesis.expected_lift.kind === "relative" + ? { + kind: "relative", + value: input.hypothesis.expected_lift.value, + } + : { + kind: "absolute", + value: input.hypothesis.expected_lift.value, + unit: input.hypothesis.expected_lift.unit, + }, + owner: { + id: input.hypothesis.owner.id, + displayName: input.hypothesis.owner.display_name, + }, + experimentScope: { + channel: input.hypothesis.experiment_scope.channel, + experimentFamilyKey: + input.hypothesis.experiment_scope.experiment_family_key, + attributionWindowHours: + input.hypothesis.experiment_scope.attribution_window_hours, + exclusionWindowHours: + input.hypothesis.experiment_scope.exclusion_window_hours, + }, + createdAt: new Date().toISOString(), + } + : undefined, }; return await this.abTestService.createTest(config); diff --git a/packages/abtest/src/hypothesis.ts b/packages/abtest/src/hypothesis.ts index d570b4ef..5c474a85 100644 --- a/packages/abtest/src/hypothesis.ts +++ b/packages/abtest/src/hypothesis.ts @@ -70,6 +70,44 @@ export class HypothesisValidationError extends Error { } } +/** + * Strict ISO 8601 validation that rejects values `Date.parse` would silently + * accept, including the year-zero string "0", localized formats like + * "01/02/03", and overflowed calendar dates like "2026-02-30". The date + * components are reconstructed and compared so overflow rolls over are caught. + */ +function isStrictIsoTimestamp(value: unknown): boolean { + if (typeof value !== "string") return false; + const re = + /^\d{4}-\d{2}-\d{2}([T ]\d{2}:\d{2}:\d{2}(\.\d{1,3})?(Z|[+-]\d{2}:\d{2})?)?$/; + if (!re.test(value)) return false; + const parts = value.split(/([T ])/); + const datePart = parts[0]; + if (!datePart) return false; + const dateNums = datePart.split("-").map(Number); + const y = dateNums[0]; + const m = dateNums[1]; + const day = dateNums[2]; + if (y === undefined || m === undefined || day === undefined) return false; + const d = new Date(Date.UTC(y, m - 1, day)); + if ( + d.getUTCFullYear() !== y || + d.getUTCMonth() !== m - 1 || + d.getUTCDate() !== day + ) { + return false; + } + // If a time component is present, ensure it parses (catches bad hours/minutes). + if (parts.length > 1) { + if (Number.isNaN(Date.parse(value))) return false; + } + return true; +} + +function isPlainObject(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + /** * Validate hypothesis metadata. When `strict` is true (launch/pre-registration), * all fields are required. When false (draft), missing fields are allowed. @@ -107,10 +145,7 @@ export function validateHypothesisMetadata( } } if (metadata.createdAt !== undefined) { - if ( - typeof metadata.createdAt !== "string" || - Number.isNaN(Date.parse(metadata.createdAt)) - ) { + if (!isStrictIsoTimestamp(metadata.createdAt)) { throw new HypothesisValidationError( `createdAt must be a valid ISO 8601 timestamp, received ${JSON.stringify(metadata.createdAt)}`, ); @@ -118,6 +153,11 @@ export function validateHypothesisMetadata( } if (metadata.primaryMetric !== undefined) { const pm = metadata.primaryMetric; + if (!isPlainObject(pm)) { + throw new HypothesisValidationError( + `primaryMetric must be an object, received ${JSON.stringify(pm)}`, + ); + } const validTypes = [ "click_rate", "conversion_rate", @@ -136,22 +176,30 @@ export function validateHypothesisMetadata( } if (metadata.expectedLift !== undefined) { const lift = metadata.expectedLift; + if (!isPlainObject(lift)) { + throw new HypothesisValidationError( + `expectedLift must be an object, received ${JSON.stringify(lift)}`, + ); + } const rawKind = (lift as { kind?: unknown }).kind; if (rawKind !== "relative" && rawKind !== "absolute") { throw new HypothesisValidationError( `expectedLift.kind must be "relative" or "absolute", received ${JSON.stringify(rawKind)}`, ); } - if (!Number.isFinite(lift.value) || lift.value <= 0) { + if (typeof lift.value !== "number" || !Number.isFinite( + lift.value, + ) || lift.value <= 0) { throw new HypothesisValidationError( - `expectedLift.value must be finite and positive, received ${lift.value}`, + `expectedLift.value must be finite and positive, received ${JSON.stringify(lift.value)}`, ); } - if (lift.kind === "absolute") { + if (rawKind === "absolute") { const validUnits = ["percentage_point", "currency_per_recipient"]; - if (!validUnits.includes(lift.unit)) { + const unit = (lift as { unit?: unknown }).unit; + if (typeof unit !== "string" || !validUnits.includes(unit)) { throw new HypothesisValidationError( - `expectedLift.unit must be one of ${validUnits.join(", ")} for absolute lift, received ${JSON.stringify(lift.unit)}`, + `expectedLift.unit must be one of ${validUnits.join(", ")} for absolute lift, received ${JSON.stringify(unit)}`, ); } } @@ -163,10 +211,11 @@ export function validateHypothesisMetadata( if ( metadata.primaryMetric !== undefined && metadata.expectedLift !== undefined && - metadata.expectedLift.kind === "absolute" + isPlainObject(metadata.expectedLift) && + (metadata.expectedLift as { kind?: unknown }).kind === "absolute" ) { const metricType = metadata.primaryMetric.type; - const unit = metadata.expectedLift.unit; + const unit = (metadata.expectedLift as { unit?: unknown }).unit; if ( metricType === "revenue_per_recipient" && unit !== "currency_per_recipient" @@ -185,7 +234,13 @@ export function validateHypothesisMetadata( } } if (metadata.owner !== undefined) { - if (!metadata.owner.id || metadata.owner.id.trim().length === 0) { + const owner = metadata.owner; + if (!isPlainObject(owner)) { + throw new HypothesisValidationError( + `owner must be an object, received ${JSON.stringify(owner)}`, + ); + } + if (typeof owner.id !== "string" || owner.id.trim().length === 0) { throw new HypothesisValidationError( "owner.id must be a non-empty string", ); @@ -193,11 +248,21 @@ export function validateHypothesisMetadata( } if (metadata.experimentScope !== undefined) { const scope = metadata.experimentScope; + if (!isPlainObject(scope)) { + throw new HypothesisValidationError( + `experimentScope must be an object, received ${JSON.stringify(scope)}`, + ); + } if (scope.channel !== "email") { throw new HypothesisValidationError( `channel must be "email", received "${scope.channel}"`, ); } + if (typeof scope.experimentFamilyKey !== "string" || scope.experimentFamilyKey.trim().length === 0) { + throw new HypothesisValidationError( + "experimentScope.experimentFamilyKey must be a non-empty string", + ); + } if (!scope.experimentFamilyKey || scope.experimentFamilyKey.trim().length === 0) { throw new HypothesisValidationError( "experimentScope.experimentFamilyKey must be a non-empty string", @@ -206,25 +271,27 @@ export function validateHypothesisMetadata( // Reject delimiter-only and empty-segment keys (".", "foo.", "foo..bar") // by requiring one or more alphanumeric segments joined by single // separators from [._-]. - if (!scope.experimentFamilyKey.match(/^[a-z0-9]+(?:[._-][a-z0-9]+)*$/)) { + if (!/^[a-z0-9]+(?:[._-][a-z0-9]+)*$/.test(scope.experimentFamilyKey)) { throw new HypothesisValidationError( `experimentFamilyKey must be dotted alphanumeric segments (e.g. "onboarding.activation"), received "${scope.experimentFamilyKey}"`, ); } if ( + typeof scope.attributionWindowHours !== "number" || !Number.isFinite(scope.attributionWindowHours) || scope.attributionWindowHours <= 0 ) { throw new HypothesisValidationError( - `attributionWindowHours must be finite and positive, received ${scope.attributionWindowHours}`, + `attributionWindowHours must be finite and positive, received ${JSON.stringify(scope.attributionWindowHours)}`, ); } if ( + typeof scope.exclusionWindowHours !== "number" || !Number.isFinite(scope.exclusionWindowHours) || scope.exclusionWindowHours < 0 ) { throw new HypothesisValidationError( - `exclusionWindowHours must be finite and non-negative, received ${scope.exclusionWindowHours}`, + `exclusionWindowHours must be finite and non-negative, received ${JSON.stringify(scope.exclusionWindowHours)}`, ); } } @@ -297,7 +364,7 @@ export function lockHypothesis( // Validate the primary metadata first so domain errors surface before // the timestamp override check; the lockedAt override is secondary input. validateHypothesisMetadata(metadata, true); - if (typeof lockedAt !== "string" || Number.isNaN(Date.parse(lockedAt))) { + if (!isStrictIsoTimestamp(lockedAt)) { throw new HypothesisValidationError( `lockedAt override must be a valid ISO 8601 timestamp, received ${JSON.stringify(lockedAt)}`, ); diff --git a/packages/abtest/src/listmonk-integration.ts b/packages/abtest/src/listmonk-integration.ts index 98e395ab..e6614202 100644 --- a/packages/abtest/src/listmonk-integration.ts +++ b/packages/abtest/src/listmonk-integration.ts @@ -16,6 +16,13 @@ import { rankMembers, type AssignmentManifest, } from "./assignment"; +import { + classifyStratum, + computeStratifiedQuotas, + DEFAULT_STRATIFICATION_POLICY, + type StratificationPolicyV1, + type StratificationResult, +} from "./stratification"; export interface ProvisionedAbTestResources { testId: string; @@ -159,6 +166,7 @@ export class ListmonkAbTestIntegration { options: { testId?: string; assignmentSeed?: string; + stratificationPolicy?: StratificationPolicyV1; } = {}, ): Promise<{ testListMappings: { variantId: string; listId: number }[]; @@ -169,6 +177,9 @@ export class ListmonkAbTestIntegration { assignmentTestId: string; audienceSnapshot: AudienceSnapshot; assignmentManifest: AssignmentManifest; + /** Stratified quota matrix computed from the resolved audience. Present + * when a stratification policy is supplied and emails are available. */ + stratification?: StratificationResult; }> { const createdListIds: number[] = []; let holdoutListId: number | undefined; @@ -304,6 +315,49 @@ export class ListmonkAbTestIntegration { }); } + // Optionally compute the recipient-domain stratified quota matrix + // from the resolved audience so each provider stratum gets a + // proportional share of every variant/holdout group. This runs the + // quota solver against real audience data and surfaces it for + // reporting/validation; the assignment itself remains the + // deterministic largest-remainder manifest above. + const stratificationPolicy = + options.stratificationPolicy ?? DEFAULT_STRATIFICATION_POLICY; + let stratification: StratificationResult | undefined; + if (stratificationPolicy.enabled) { + const emailsAvailable = resolvedMembers.some( + (member) => member.email !== undefined, + ); + if (emailsAvailable) { + // Build stratum sizes by classifying each member's domain. + const stratumSizes: Record = {}; + for (const member of resolvedMembers) { + const stratum = classifyStratum( + member.email ?? "", + stratificationPolicy, + ); + stratumSizes[stratum] = (stratumSizes[stratum] ?? 0) + 1; + } + // Build exact group counts from the manifest groups. + const groupExactCounts: Record = {}; + const groupOrder: string[] = []; + for (const group of assignmentManifest.groups) { + const key = + group.kind === "variant" + ? `variant:${group.variantId ?? ""}` + : "holdout"; + groupOrder.push(key); + groupExactCounts[key] = group.expectedCount; + } + stratification = computeStratifiedQuotas({ + stratumSizes, + groupExactCounts, + groupOrder, + totalAudience: resolvedSnapshot.subscriberCount, + }); + } + } + return { testListMappings, holdoutListId, @@ -313,6 +367,7 @@ export class ListmonkAbTestIntegration { assignmentTestId: testId, audienceSnapshot: resolvedSnapshot, assignmentManifest, + stratification, }; } catch (error) { await this.deleteListsBestEffort([...createdListIds].reverse()); diff --git a/packages/abtest/src/operations.ts b/packages/abtest/src/operations.ts index bf6e8e2e..f56540dc 100644 --- a/packages/abtest/src/operations.ts +++ b/packages/abtest/src/operations.ts @@ -202,6 +202,21 @@ const abTestSchema = z.object({ checksum: z.string().optional(), }) .optional(), + // Recipient-domain stratified quota matrix, when produced during provisioning. + stratification: z + .object({ + quotas: z.record(z.string(), z.record(z.string(), z.number())), + cells: z.array( + z.object({ + stratumKey: z.string(), + groupKey: z.string(), + quota: z.number(), + ideal: z.number(), + }), + ), + stratumSizes: z.record(z.string(), z.number()), + }) + .optional(), }); const testResultsSchema = z.object({ @@ -304,6 +319,43 @@ const createAbTestInputSchema = z.object({ auto_launch: optionalBooleanSchema, auto_deploy_winner: optionalBooleanSchema, ignore_sample_size_warnings: optionalBooleanSchema, + hypothesis: z + .object({ + objective: z.string().min(1), + hypothesis: z.string().min(1), + primary_metric: z.object({ + type: z.enum([ + "click_rate", + "conversion_rate", + "revenue_per_recipient", + ]), + direction: z.enum(["maximize", "minimize"]), + }), + expected_lift: z.union([ + z.object({ + kind: z.literal("relative"), + value: z.number().finite().positive(), + }), + z.object({ + kind: z.literal("absolute"), + value: z.number().finite().positive(), + unit: z.enum(["percentage_point", "currency_per_recipient"]), + }), + ]), + owner: z.object({ + id: z.string().min(1), + display_name: z.string().optional(), + }), + experiment_scope: z.object({ + channel: z.literal("email"), + experiment_family_key: z + .string() + .regex(/^[a-z0-9]+(?:[._-][a-z0-9]+)*$/), + attribution_window_hours: z.number().finite().positive(), + exclusion_window_hours: z.number().finite().nonnegative(), + }), + }) + .optional(), }); const analyzeAbTestInputSchema = testIdInputSchema.extend({ diff --git a/packages/abtest/src/persistence.ts b/packages/abtest/src/persistence.ts index ed5e22ae..6a70a5eb 100644 --- a/packages/abtest/src/persistence.ts +++ b/packages/abtest/src/persistence.ts @@ -243,7 +243,13 @@ function isStoredAbTest(value: unknown): boolean { // Pre-registration hypothesis: optional, but the nested shape and the // locked-state checksum invariant are validated when present so that // loadStoredAbTests never hydrates malformed or tampered metadata. - (value.hypothesis === undefined || isStoredHypothesis(value.hypothesis)) + (value.hypothesis === undefined || isStoredHypothesis(value.hypothesis)) && + // If an assignment manifest exists, the hypothesis must be present and + // locked with a valid checksum. This enforces the pre-registration + // guarantee that hypothesis content cannot change after recipient + // assignment. + (value.assignmentManifest === undefined || + (value.hypothesis !== undefined && isStoredHypothesis(value.hypothesis))) ); } diff --git a/packages/abtest/src/stratification.ts b/packages/abtest/src/stratification.ts index 9473e809..9bfea6e8 100644 --- a/packages/abtest/src/stratification.ts +++ b/packages/abtest/src/stratification.ts @@ -65,9 +65,32 @@ export function normalizeDomain(email: string): string { return domain; } +/** + * Build a domain-to-provider lookup map by normalizing each configured + * domain with the same rules applied to subscriber emails. This ensures a + * mixed-case or trailing-dot entry in the providerDomainMap (e.g. + * "GMAIL.COM") still classifies matching subscribers correctly. + */ +function buildProviderLookup( + policy: StratificationPolicyV1, +): Map { + const lookup = new Map(); + for (const [provider, domains] of Object.entries(policy.providerDomainMap)) { + for (const rawDomain of domains) { + const normalized = normalizeDomain(`@${rawDomain}`); + if (normalized !== "") { + lookup.set(normalized, provider); + } + } + } + return lookup; +} + /** * Classify a subscriber's email domain into a stratum key using - * the provider domain map. + * the provider domain map. Configured domains are normalized with the same + * rules applied to subscriber emails so mixed-case or trailing-dot entries + * match correctly. */ export function classifyStratum( email: string, @@ -77,10 +100,10 @@ export function classifyStratum( if (domain === "") { return policy.unknownStratumKey; } - for (const [provider, domains] of Object.entries(policy.providerDomainMap)) { - if (domains.includes(domain)) { - return provider; - } + const lookup = buildProviderLookup(policy); + const provider = lookup.get(domain); + if (provider !== undefined) { + return provider; } return policy.otherStratumKey; } @@ -229,23 +252,47 @@ export function computeStratifiedQuotas(params: { if (!deficitGroup || !surplusGroup) break; // Choose the row where the deficit cell is most below ideal and the - // surplus cell can donate (quota > 0). This keeps cell deviations - // minimal and avoids negative quotas. + // surplus cell can donate. We prefer swaps that keep cells within their + // floor-or-ceiling allocation: the donor stays at/above its floor and + // the receiver stays at/below its ceiling. When no such bounded swap is + // possible (column totals may require a cell outside the naive floor/ + // ceiling band), fall back to the row that minimizes how far outside + // the band the resulting cells would land. let bestStratum: string | null = null; let bestScore = -Infinity; + let bestBounded = false; for (const sk of stratumKeys) { const row = quotas[sk]; if (!row) continue; const surplusQuota = row[surplusGroup] ?? 0; + const deficitQuota = row[deficitGroup] ?? 0; + const surplusIdeal = idealLookup.get(`${sk}:${surplusGroup}`) ?? 0; + const deficitIdeal = idealLookup.get(`${sk}:${deficitGroup}`) ?? 0; + const surplusBounded = surplusQuota > Math.floor(surplusIdeal); + const deficitBounded = deficitQuota < Math.ceil(deficitIdeal); + const bounded = surplusBounded && deficitBounded; + // Always require a positive donor and a receiver below ceiling so + // the swap is physically valid (no negative quota, no receiver + // already at ceiling that the swap would exceed). if (surplusQuota <= 0) continue; - const deficitDev = cellDeviation(sk, deficitGroup); - const surplusDev = cellDeviation(sk, surplusGroup); - // Prefer rows where the deficit cell is far below ideal and the - // surplus cell is far above ideal. + if (deficitQuota >= Math.ceil(deficitIdeal)) continue; + const deficitDev = deficitQuota - deficitIdeal; + const surplusDev = surplusQuota - surplusIdeal; const score = surplusDev - deficitDev; - if (score > bestScore) { + // Prefer bounded swaps; among the same boundedness, prefer the + // best score. + if (bestStratum === null) { + bestStratum = sk; bestScore = score; + bestBounded = bounded; + } else if (bounded && !bestBounded) { + // A bounded swap beats an unbounded incumbent. bestStratum = sk; + bestScore = score; + bestBounded = bounded; + } else if (bounded === bestBounded && score > bestScore) { + bestStratum = sk; + bestScore = score; } } if (!bestStratum) break; diff --git a/packages/abtest/src/types.ts b/packages/abtest/src/types.ts index 6c83c776..42ad2f7e 100644 --- a/packages/abtest/src/types.ts +++ b/packages/abtest/src/types.ts @@ -91,6 +91,9 @@ export interface AbTest { hypothesis?: import("./hypothesis").HypothesisMetadata; /** Assignment provenance: whether the test has a deterministic manifest. */ assignmentProvenance?: "manifest_v1" | "legacy_unavailable"; + /** Recipient-domain stratified quota matrix, computed during provisioning + * when a stratification policy is enabled and emails are available. */ + stratification?: import("./stratification").StratificationResult; /** * Deterministic assignment manifest produced from the seed + audience. * Once stored, retries and reconciliation reuse it rather than @@ -207,6 +210,10 @@ export interface AbTestConfig { // Orchestration settings (stage 3) durationHours?: number; // Planned test duration in hours launchAt?: string; // ISO timestamp for scheduled launch + // Pre-registration hypothesis (advanced experimentation). Optional; when + // provided unlocked, createTest locks it before provisioning so the + // assignment manifest cannot be separated from a frozen hypothesis. + hypothesis?: import("./hypothesis").HypothesisMetadata; } export interface AbTestInput { @@ -240,6 +247,31 @@ export interface CreateAbTestInput { launch_at?: string; // ISO timestamp for scheduled launch auto_deploy_winner?: boolean; // Auto-deploy to holdout group (holdout mode only) ignore_sample_size_warnings?: boolean; // Skip sample size validation warnings + // Pre-registration hypothesis. Operators describe the objective, primary + // metric, expected lift, owner, and experiment scope; the service locks it + // before assignment so the metadata cannot change after recipients are set. + hypothesis?: { + objective: string; + hypothesis: string; + primary_metric: { + type: "click_rate" | "conversion_rate" | "revenue_per_recipient"; + direction: "maximize" | "minimize"; + }; + expected_lift: + | { kind: "relative"; value: number } + | { + kind: "absolute"; + value: number; + unit: "percentage_point" | "currency_per_recipient"; + }; + owner: { id: string; display_name?: string }; + experiment_scope: { + channel: "email"; + experiment_family_key: string; + attribution_window_hours: number; + exclusion_window_hours: number; + }; + }; } export interface AnalyzeAbTestInput { diff --git a/packages/abtest/tests/basic.test.ts b/packages/abtest/tests/basic.test.ts index 0bdbef50..e7b093f3 100644 --- a/packages/abtest/tests/basic.test.ts +++ b/packages/abtest/tests/basic.test.ts @@ -28,6 +28,50 @@ test("CreateAbTestCommand uses provided campaign_id", async () => { expect(created.status).toBe("draft"); }); +test("CreateAbTestCommand locks a provided hypothesis before provisioning", async () => { + const service = new AbTestService(); + const command = new CreateAbTestCommand(service); + + const created = await command.execute({ + name: "Hypothesis Wiring", + campaign_id: "campaign-456", + lists: [1], + variants: [ + { + name: "A", + percentage: 50, + campaign_config: { subject: "A", body: "Body A" }, + }, + { + name: "B", + percentage: 50, + campaign_config: { subject: "B", body: "Body B" }, + }, + ], + hypothesis: { + objective: "Increase CTR", + hypothesis: "Shorter subject lifts CTR", + primary_metric: { + type: "click_rate", + direction: "maximize", + }, + expected_lift: { kind: "relative", value: 0.1 }, + owner: { id: "user-1" }, + experiment_scope: { + channel: "email", + experiment_family_key: "onboarding.welcome", + attribution_window_hours: 72, + exclusion_window_hours: 168, + }, + }, + }); + + expect(created.hypothesis).toBeDefined(); + expect(created.hypothesis?.lockedAt).toBeDefined(); + expect(created.hypothesis?.checksum).toMatch(/^[0-9a-f]{64}$/); + expect(created.assignmentProvenance).toBeUndefined(); +}); + test("analyzeStatisticalSignificance returns stable values on zero samples", async () => { const service = new AbTestService(); diff --git a/packages/abtest/tests/hypothesis.test.ts b/packages/abtest/tests/hypothesis.test.ts index 7ff970ea..fa46fa86 100644 --- a/packages/abtest/tests/hypothesis.test.ts +++ b/packages/abtest/tests/hypothesis.test.ts @@ -142,6 +142,56 @@ describe("validateHypothesisMetadata", () => { ).toThrow(HypothesisValidationError); }); + it("rejects non-ISO timestamps that Date.parse would accept", () => { + // "0", "01/02/03", and overflowed "2026-02-30" must be rejected. + for (const bad of ["0", "01/02/03", "2026-02-30", "2026-13-40"]) { + expect(() => + validateHypothesisMetadata(makeHypothesis({ createdAt: bad })), + ).toThrow(HypothesisValidationError); + } + }); + + it("rejects null nested metadata with a validation error", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + primaryMetric: null as unknown as HypothesisMetadata["primaryMetric"], + }), + ), + ).toThrow(HypothesisValidationError); + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + expectedLift: null as unknown as HypothesisMetadata["expectedLift"], + }), + ), + ).toThrow(HypothesisValidationError); + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + owner: null as unknown as HypothesisMetadata["owner"], + }), + ), + ).toThrow(HypothesisValidationError); + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + experimentScope: null as unknown as HypothesisMetadata["experimentScope"], + }), + ), + ).toThrow(HypothesisValidationError); + }); + + it("rejects non-string owner id with a validation error", () => { + expect(() => + validateHypothesisMetadata( + makeHypothesis({ + owner: { id: 123 as unknown as string }, + }), + ), + ).toThrow(HypothesisValidationError); + }); + it("rejects invalid primaryMetric.type in strict mode", () => { expect(() => validateHypothesisMetadata( diff --git a/packages/abtest/tests/stratification.test.ts b/packages/abtest/tests/stratification.test.ts index 850c0984..8d2d11e5 100644 --- a/packages/abtest/tests/stratification.test.ts +++ b/packages/abtest/tests/stratification.test.ts @@ -48,6 +48,15 @@ describe("classifyStratum", () => { it("handles case-insensitive domains", () => { expect(classifyStratum("User@GMAIL.COM", policy)).toBe("gmail"); }); + + it("normalizes mixed-case configured domains", () => { + const mixedPolicy: typeof policy = { + ...policy, + providerDomainMap: { google: ["GMAIL.COM."] }, + }; + expect(classifyStratum("user@gmail.com", mixedPolicy)).toBe("google"); + expect(classifyStratum("user@Gmail.Com", mixedPolicy)).toBe("google"); + }); }); describe("computeStratifiedQuotas", () => { @@ -160,4 +169,37 @@ describe("computeStratifiedQuotas", () => { expect(cell.quota).toBeLessThanOrEqual(ceilIdeal); } }); + + it("matches exact row/column sums for the codex 4x4 example", () => { + // The case ocr flagged: strata {s0:116,s1:105,s2:74,s3:47} and groups + // {g0:37,g1:216,g2:63,g3:26}. The biproportional allocation must match + // both row and column totals exactly. Cells stay close to their ideal + // (within 1 of floor/ceil where possible); a column may force one cell + // outside the naive floor/ceil band to satisfy the exact count. + const result = computeStratifiedQuotas({ + stratumSizes: { s0: 116, s1: 105, s2: 74, s3: 47 }, + groupExactCounts: { g0: 37, g1: 216, g2: 63, g3: 26 }, + groupOrder: ["g0", "g1", "g2", "g3"], + totalAudience: 342, + }); + for (const [sk, row] of Object.entries(result.quotas)) { + const rowSum = Object.values(row).reduce((s, n) => s + n, 0); + expect(rowSum).toBe( + ({ s0: 116, s1: 105, s2: 74, s3: 47 } as Record)[sk], + ); + } + for (const gk of ["g0", "g1", "g2", "g3"]) { + const colSum = Object.values(result.quotas).reduce( + (s, row) => s + (row[gk] ?? 0), + 0, + ); + expect(colSum).toBe( + ({ g0: 37, g1: 216, g2: 63, g3: 26 } as Record)[gk], + ); + } + // Each cell stays within 1 of its ideal (no runaway deviations). + for (const cell of result.cells) { + expect(Math.abs(cell.quota - cell.ideal)).toBeLessThanOrEqual(1.5); + } + }); }); From e79acecbc646bdc752de5c55b0f6b576c56cc933 Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 12:52:27 +0900 Subject: [PATCH 07/20] fix(abtest): isolate stratification from provisioning failure path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenCodeReview findings on the wiring commit: - high: stratification was inside the try/catch that calls deleteListsBestEffort, so a quota invariant failure would cascade into deleting all provisioned lists and tearing down the test. Wrap the computation in its own try/catch so a failure degrades gracefully to an undefined stratification. - medium: emailsAvailable used .some(), so a single member with email classified all members — email-less subscribers were silently bucketed as "unknown". Require every member to carry an email before computing the matrix. - low: combined the email-availability check and classification into a single pass over resolvedMembers. --- packages/abtest/src/listmonk-integration.ts | 77 ++++++++++++--------- 1 file changed, 44 insertions(+), 33 deletions(-) diff --git a/packages/abtest/src/listmonk-integration.ts b/packages/abtest/src/listmonk-integration.ts index e6614202..8b901021 100644 --- a/packages/abtest/src/listmonk-integration.ts +++ b/packages/abtest/src/listmonk-integration.ts @@ -317,44 +317,55 @@ export class ListmonkAbTestIntegration { // Optionally compute the recipient-domain stratified quota matrix // from the resolved audience so each provider stratum gets a - // proportional share of every variant/holdout group. This runs the - // quota solver against real audience data and surfaces it for - // reporting/validation; the assignment itself remains the - // deterministic largest-remainder manifest above. + // proportional share of every variant/holdout group. This is a + // reporting/validation enrichment; the assignment itself remains + // the deterministic largest-remainder manifest above. A failure + // here must not tear down provisioning, so it is isolated in its + // own try/catch and degrades to an undefined stratification. const stratificationPolicy = options.stratificationPolicy ?? DEFAULT_STRATIFICATION_POLICY; let stratification: StratificationResult | undefined; if (stratificationPolicy.enabled) { - const emailsAvailable = resolvedMembers.some( - (member) => member.email !== undefined, - ); - if (emailsAvailable) { - // Build stratum sizes by classifying each member's domain. - const stratumSizes: Record = {}; - for (const member of resolvedMembers) { - const stratum = classifyStratum( - member.email ?? "", - stratificationPolicy, - ); - stratumSizes[stratum] = (stratumSizes[stratum] ?? 0) + 1; - } - // Build exact group counts from the manifest groups. - const groupExactCounts: Record = {}; - const groupOrder: string[] = []; - for (const group of assignmentManifest.groups) { - const key = - group.kind === "variant" - ? `variant:${group.variantId ?? ""}` - : "holdout"; - groupOrder.push(key); - groupExactCounts[key] = group.expectedCount; + // Require every member to carry an email so the quota matrix + // reflects the full audience. Partial coverage would silently + // bucket email-less subscribers into "unknown" and skew the + // proportions. + const allMembersHaveEmail = + resolvedMembers.length > 0 && + resolvedMembers.every((member) => member.email !== undefined); + if (allMembersHaveEmail) { + try { + // Single pass: classify each member and tally stratum sizes. + const stratumSizes: Record = {}; + for (const member of resolvedMembers) { + const stratum = classifyStratum( + member.email ?? "", + stratificationPolicy, + ); + stratumSizes[stratum] = (stratumSizes[stratum] ?? 0) + 1; + } + // Build exact group counts from the manifest groups. + const groupExactCounts: Record = {}; + const groupOrder: string[] = []; + for (const group of assignmentManifest.groups) { + const key = + group.kind === "variant" + ? `variant:${group.variantId ?? ""}` + : "holdout"; + groupOrder.push(key); + groupExactCounts[key] = group.expectedCount; + } + stratification = computeStratifiedQuotas({ + stratumSizes, + groupExactCounts, + groupOrder, + totalAudience: resolvedSnapshot.subscriberCount, + }); + } catch { + // Stratification is non-critical; leave it undefined so + // provisioning proceeds with the manifest assignment. + stratification = undefined; } - stratification = computeStratifiedQuotas({ - stratumSizes, - groupExactCounts, - groupOrder, - totalAudience: resolvedSnapshot.subscriberCount, - }); } } From 853d6bf81f0131ae1dcfa31eae603078ac37a194 Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 13:08:20 +0900 Subject: [PATCH 08/20] fix(abtest): preserve legacy manifests, harden stratification validation Addresses the third @codex review findings on PR #46. P1: - The manifest+lock invariant retroactively rejected existing v2 records that carry an assignmentManifest but predate hypothesis pre-registration, breaking list/get after a successful create. The invariant now applies only when BOTH manifest and hypothesis are present, so legacy manifest-only records still load. Added a regression test. P2: - computeStratifiedQuotas now rejects a non-positive totalAudience before dividing, so an all-zero input cannot produce NaN ideals/quota cells. - createTest verifies the checksum of a caller-supplied pre-locked hypothesis before accepting it, so tampered metadata cannot reach remote provisioning. - Added createStratumClassifier that builds the provider-domain lookup once; the integration classifies a large audience without rebuilding the map per recipient. - Added isStoredStratification so corrupt stratification state (negative quotas, malformed cells) is rejected at the file boundary. Tests cover the legacy manifest-only acceptance, the positive-audience guard, and the all-zero rejection. --- packages/abtest/src/abtest-service.ts | 14 +++- packages/abtest/src/listmonk-integration.ts | 11 ++- packages/abtest/src/persistence.ts | 77 ++++++++++++++++++-- packages/abtest/src/stratification.ts | 48 +++++++++--- packages/abtest/tests/persistence.test.ts | 33 +++++++++ packages/abtest/tests/stratification.test.ts | 2 +- 6 files changed, 161 insertions(+), 24 deletions(-) diff --git a/packages/abtest/src/abtest-service.ts b/packages/abtest/src/abtest-service.ts index 1e604e99..5e14bf99 100644 --- a/packages/abtest/src/abtest-service.ts +++ b/packages/abtest/src/abtest-service.ts @@ -11,7 +11,7 @@ import { DEFAULT_STATISTICAL_POLICY, fixedHorizonGate, } from "./statistics"; -import { lockHypothesis } from "./hypothesis"; +import { lockHypothesis, verifyHypothesisChecksum } from "./hypothesis"; import type { AbTest, AbTestConfig, @@ -181,9 +181,19 @@ export class AbTestService { testListMappings: [], // Lock the pre-registration hypothesis before any provisioning so the // assignment manifest is bound to a frozen, checksummed hypothesis. + // When a caller supplies an already-locked hypothesis, verify its + // checksum before accepting it, so tampered metadata cannot reach + // remote campaign/list provisioning. hypothesis: config.hypothesis ? config.hypothesis.lockedAt - ? config.hypothesis + ? (() => { + if (!verifyHypothesisChecksum(config.hypothesis!)) { + throw new Error( + "Pre-locked hypothesis checksum verification failed; the metadata may have been tampered with", + ); + } + return config.hypothesis!; + })() : lockHypothesis(config.hypothesis) : undefined, }; diff --git a/packages/abtest/src/listmonk-integration.ts b/packages/abtest/src/listmonk-integration.ts index 8b901021..ce9c4889 100644 --- a/packages/abtest/src/listmonk-integration.ts +++ b/packages/abtest/src/listmonk-integration.ts @@ -17,8 +17,8 @@ import { type AssignmentManifest, } from "./assignment"; import { - classifyStratum, computeStratifiedQuotas, + createStratumClassifier, DEFAULT_STRATIFICATION_POLICY, type StratificationPolicyV1, type StratificationResult, @@ -335,13 +335,12 @@ export class ListmonkAbTestIntegration { resolvedMembers.every((member) => member.email !== undefined); if (allMembersHaveEmail) { try { - // Single pass: classify each member and tally stratum sizes. + // Build the provider-domain lookup once and classify each + // member in a single pass, tallying stratum sizes. + const classifier = createStratumClassifier(stratificationPolicy); const stratumSizes: Record = {}; for (const member of resolvedMembers) { - const stratum = classifyStratum( - member.email ?? "", - stratificationPolicy, - ); + const stratum = classifier(member.email ?? ""); stratumSizes[stratum] = (stratumSizes[stratum] ?? 0) + 1; } // Build exact group counts from the manifest groups. diff --git a/packages/abtest/src/persistence.ts b/packages/abtest/src/persistence.ts index 6a70a5eb..c20f04a3 100644 --- a/packages/abtest/src/persistence.ts +++ b/packages/abtest/src/persistence.ts @@ -244,15 +244,82 @@ function isStoredAbTest(value: unknown): boolean { // locked-state checksum invariant are validated when present so that // loadStoredAbTests never hydrates malformed or tampered metadata. (value.hypothesis === undefined || isStoredHypothesis(value.hypothesis)) && - // If an assignment manifest exists, the hypothesis must be present and - // locked with a valid checksum. This enforces the pre-registration - // guarantee that hypothesis content cannot change after recipient - // assignment. + // When BOTH a manifest and a hypothesis are present, the hypothesis + // must be locked. This enforces the pre-registration guarantee for new + // records without retroactively rejecting legacy v2 records that carry + // a manifest but predate hypothesis pre-registration. (value.assignmentManifest === undefined || - (value.hypothesis !== undefined && isStoredHypothesis(value.hypothesis))) + value.hypothesis === undefined || + (isRecord(value.hypothesis) && + value.hypothesis.lockedAt !== undefined && + isStoredHypothesis(value.hypothesis))) && + // Stratification quota matrix: optional, structurally validated when + // present so corrupt state (negative quotas, malformed cells) is + // rejected at the file boundary. + (value.stratification === undefined || + isStoredStratification(value.stratification)) ); } +/** + * Validate a persisted stratification quota matrix. Requires non-negative + * quotas and ideals, and that every cell references a known stratum/group. + */ +function isStoredStratification(value: unknown): boolean { + if (!isRecord(value)) { + return false; + } + const quotas = value.quotas; + const cells = value.cells; + const stratumSizes = value.stratumSizes; + if (!isRecord(quotas) || !Array.isArray(cells) || !isRecord(stratumSizes)) { + return false; + } + // Every quota row must map group keys to non-negative finite numbers. + for (const row of Object.values(quotas)) { + if (!isRecord(row)) return false; + for (const n of Object.values(row)) { + if ( + typeof n !== "number" || + !Number.isFinite(n) || + n < 0 || + !Number.isInteger(n) + ) { + return false; + } + } + } + // stratumSizes must be non-negative integers. + for (const n of Object.values(stratumSizes)) { + if ( + typeof n !== "number" || + !Number.isFinite(n) || + n < 0 || + !Number.isInteger(n) + ) { + return false; + } + } + // Each cell must have the required shape with non-negative values. + for (const cell of cells) { + if ( + !isRecord(cell) || + typeof cell.stratumKey !== "string" || + typeof cell.groupKey !== "string" || + typeof cell.quota !== "number" || + !Number.isFinite(cell.quota) || + cell.quota < 0 || + !Number.isInteger(cell.quota) || + typeof cell.ideal !== "number" || + !Number.isFinite(cell.ideal) || + cell.ideal < 0 + ) { + return false; + } + } + return true; +} + const HYPOTHESIS_METRIC_TYPES = new Set([ "click_rate", "conversion_rate", diff --git a/packages/abtest/src/stratification.ts b/packages/abtest/src/stratification.ts index 9bfea6e8..abe42397 100644 --- a/packages/abtest/src/stratification.ts +++ b/packages/abtest/src/stratification.ts @@ -86,26 +86,46 @@ function buildProviderLookup( return lookup; } +/** + * Build a reusable stratum classifier for a policy. The provider-domain + * lookup map is built once, so classifying a large audience avoids + * rebuilding it for every subscriber. The returned function has the same + * semantics as {@link classifyStratum}. + */ +export function createStratumClassifier( + policy: StratificationPolicyV1, +): (email: string) => string { + const lookup = buildProviderLookup(policy); + const unknownKey = policy.unknownStratumKey; + const otherKey = policy.otherStratumKey; + return (email: string): string => { + const domain = normalizeDomain(email); + if (domain === "") { + return unknownKey; + } + const provider = lookup.get(domain); + if (provider !== undefined) { + return provider; + } + return otherKey; + }; +} + /** * Classify a subscriber's email domain into a stratum key using * the provider domain map. Configured domains are normalized with the same * rules applied to subscriber emails so mixed-case or trailing-dot entries * match correctly. + * + * For large audiences, prefer {@link createStratumClassifier} to build the + * provider lookup once and avoid rebuilding it per subscriber. */ export function classifyStratum( email: string, policy: StratificationPolicyV1, ): string { - const domain = normalizeDomain(email); - if (domain === "") { - return policy.unknownStratumKey; - } - const lookup = buildProviderLookup(policy); - const provider = lookup.get(domain); - if (provider !== undefined) { - return provider; - } - return policy.otherStratumKey; + const classifier = createStratumClassifier(policy); + return classifier(email); } export interface StratumQuotaCell { @@ -153,6 +173,14 @@ export function computeStratifiedQuotas(params: { `Stratified quota invariant: strata sum ${totalFromStrata} != groups sum ${totalFromGroups}`, ); } + if (totalAudience <= 0) { + // An empty audience (all-zero strata/groups) would pass the equality + // checks above but produce NaN ideals via 0/0 division. Reject it + // explicitly so callers cannot persist NaN quota matrices. + throw new Error( + `Stratified quota invariant: totalAudience must be positive, received ${totalAudience}`, + ); + } if (totalAudience !== totalFromStrata) { // totalAudience is the divisor for proportional ideals; a mismatch // (including zero) silently skews proportions or yields NaN cells. diff --git a/packages/abtest/tests/persistence.test.ts b/packages/abtest/tests/persistence.test.ts index 75ff9044..a6634a40 100644 --- a/packages/abtest/tests/persistence.test.ts +++ b/packages/abtest/tests/persistence.test.ts @@ -377,5 +377,38 @@ describe("A/B test persistence", () => { await expect(loadStoredAbTests(storePath)).rejects.toThrow( "test 0 failed schema validation", ); + + // A legacy v2 record with an assignment manifest but no hypothesis + // (predating pre-registration) must still load — the manifest+lock + // invariant applies only when BOTH are present. + const legacyWithManifest = { + ...validTest, + assignmentManifest: { + algorithm: "sha256-order-largest-remainder-v1", + seed: "seed-1", + audienceChecksum: "abc", + groups: [ + { + kind: "variant" as const, + variantId: "v1", + expectedCount: 50, + subscriberChecksum: "x", + }, + { + kind: "holdout" as const, + expectedCount: 50, + subscriberChecksum: "y", + }, + ], + assignedCount: 100, + }, + assignmentProvenance: "manifest_v1" as const, + }; + await writeFile( + storePath, + `${JSON.stringify({ version: 1, tests: [legacyWithManifest] })}\n`, + "utf8", + ); + await expect(loadStoredAbTests(storePath)).resolves.toHaveLength(1); }); }); diff --git a/packages/abtest/tests/stratification.test.ts b/packages/abtest/tests/stratification.test.ts index 8d2d11e5..1b2c5ba2 100644 --- a/packages/abtest/tests/stratification.test.ts +++ b/packages/abtest/tests/stratification.test.ts @@ -152,7 +152,7 @@ describe("computeStratifiedQuotas", () => { groupOrder: ["variant:A", "variant:B"], totalAudience: 0, }), - ).toThrow("totalAudience 0 != strata sum 1000"); + ).toThrow("totalAudience must be positive, received 0"); }); it("each cell is floor or ceil of ideal", () => { From 54d895b5a1ed3b6e7381643b3a9b39d2e775376f Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 13:20:56 +0900 Subject: [PATCH 09/20] fix(abtest): export classifier, validate quota components, verify pre-lock Addresses the cheaper findings from the fourth @codex review: - Export createStratumClassifier from the package entry point so consumers can build the provider lookup once. - computeStratifiedQuotas validates every stratum size and group count is a non-negative integer before summing, so fractional/negative components cannot hide behind a valid total. - createTest validates a pre-locked hypothesis strictly (not just its checksum) before accepting it, rejecting malformed locked metadata. The two deeper P1 findings (apply stratified quotas to actual assignment slices, and bind the hypothesis checksum to the assignment manifest) are tracked as separate Change Set C/D work: they require a stratification-aware assignment algorithm and a manifest-checksum schema extension respectively, which are out of scope for this module-and-wiring PR. --- packages/abtest/src/abtest-service.ts | 13 +++++++++---- packages/abtest/src/index.ts | 1 + packages/abtest/src/stratification.ts | 20 ++++++++++++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/packages/abtest/src/abtest-service.ts b/packages/abtest/src/abtest-service.ts index 5e14bf99..7895f85d 100644 --- a/packages/abtest/src/abtest-service.ts +++ b/packages/abtest/src/abtest-service.ts @@ -11,7 +11,11 @@ import { DEFAULT_STATISTICAL_POLICY, fixedHorizonGate, } from "./statistics"; -import { lockHypothesis, verifyHypothesisChecksum } from "./hypothesis"; +import { + lockHypothesis, + validateHypothesisMetadata, + verifyHypothesisChecksum, +} from "./hypothesis"; import type { AbTest, AbTestConfig, @@ -181,12 +185,13 @@ export class AbTestService { testListMappings: [], // Lock the pre-registration hypothesis before any provisioning so the // assignment manifest is bound to a frozen, checksummed hypothesis. - // When a caller supplies an already-locked hypothesis, verify its - // checksum before accepting it, so tampered metadata cannot reach - // remote campaign/list provisioning. + // When a caller supplies an already-locked hypothesis, validate it + // strictly and verify its checksum before accepting it, so tampered + // or malformed metadata cannot reach remote campaign/list provisioning. hypothesis: config.hypothesis ? config.hypothesis.lockedAt ? (() => { + validateHypothesisMetadata(config.hypothesis!, true); if (!verifyHypothesisChecksum(config.hypothesis!)) { throw new Error( "Pre-locked hypothesis checksum verification failed; the metadata may have been tampered with", diff --git a/packages/abtest/src/index.ts b/packages/abtest/src/index.ts index 8c6eba64..331582a1 100644 --- a/packages/abtest/src/index.ts +++ b/packages/abtest/src/index.ts @@ -53,6 +53,7 @@ export { export { classifyStratum, computeStratifiedQuotas, + createStratumClassifier, DEFAULT_STRATIFICATION_POLICY, normalizeDomain, type StratificationPolicyV1, diff --git a/packages/abtest/src/stratification.ts b/packages/abtest/src/stratification.ts index abe42397..08e56c40 100644 --- a/packages/abtest/src/stratification.ts +++ b/packages/abtest/src/stratification.ts @@ -160,6 +160,26 @@ export function computeStratifiedQuotas(params: { const { stratumSizes, groupExactCounts, groupOrder, totalAudience } = params; + // Validate that every stratum size and group count is a non-negative + // integer before summing, so fractional or negative components cannot + // produce a "valid" total that hides malformed input. + const isNonNegInt = (n: unknown): boolean => + typeof n === "number" && Number.isInteger(n) && n >= 0; + for (const [k, v] of Object.entries(stratumSizes)) { + if (!isNonNegInt(v)) { + throw new Error( + `Stratified quota invariant: stratum "${k}" size must be a non-negative integer, received ${JSON.stringify(v)}`, + ); + } + } + for (const [k, v] of Object.entries(groupExactCounts)) { + if (!isNonNegInt(v)) { + throw new Error( + `Stratified quota invariant: group "${k}" count must be a non-negative integer, received ${JSON.stringify(v)}`, + ); + } + } + const totalFromStrata = Object.values(stratumSizes).reduce( (sum, n) => sum + n, 0, From 48bf2c8686dba42cb2678f02aa5541bb3bf02143 Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 13:35:29 +0900 Subject: [PATCH 10/20] fix(abtest): forward stratification policy from create flows Addresses the @codex P1 finding on commit 54d895b: the production path always passed { testId } and DEFAULT_STRATIFICATION_POLICY.enabled is false, so CLI/MCP holdout creation could never compute AbTest.stratification. - CreateAbTestInput gains enable_stratification; AbTestConfig gains stratificationPolicy. - basic.ts maps enable_stratification to the default policy with enabled=true. - AbTestService.createTest forwards config.stratificationPolicy to segmentSubscribersForHoldout, so an enabled policy reaches the quota computation. - createAbTestInputSchema exposes enable_stratification to CLI/MCP. --- packages/abtest/src/abtest-service.ts | 5 ++++- packages/abtest/src/basic.ts | 4 ++++ packages/abtest/src/operations.ts | 3 +++ packages/abtest/src/types.ts | 10 ++++++++++ 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/abtest/src/abtest-service.ts b/packages/abtest/src/abtest-service.ts index 7895f85d..d74d0897 100644 --- a/packages/abtest/src/abtest-service.ts +++ b/packages/abtest/src/abtest-service.ts @@ -235,7 +235,10 @@ export class AbTestService { config.baseConfig.lists, variants, testGroupPercentage, - { testId: abTest.id }, + { + testId: abTest.id, + stratificationPolicy: config.stratificationPolicy, + }, ); testListMappings = segmentationResult.testListMappings; diff --git a/packages/abtest/src/basic.ts b/packages/abtest/src/basic.ts index 1024efdb..37be6ec7 100644 --- a/packages/abtest/src/basic.ts +++ b/packages/abtest/src/basic.ts @@ -8,6 +8,7 @@ import type { CreateAbTestInput, TestAnalysis, } from "./types"; +import { DEFAULT_STRATIFICATION_POLICY } from "./stratification"; // Simple A/B Test command wrappers (no longer extending BaseCommand) export class CreateAbTestCommand { @@ -83,6 +84,9 @@ export class CreateAbTestCommand { createdAt: new Date().toISOString(), } : undefined, + stratificationPolicy: input.enable_stratification + ? { ...DEFAULT_STRATIFICATION_POLICY, enabled: true } + : undefined, }; return await this.abTestService.createTest(config); diff --git a/packages/abtest/src/operations.ts b/packages/abtest/src/operations.ts index f56540dc..911772dc 100644 --- a/packages/abtest/src/operations.ts +++ b/packages/abtest/src/operations.ts @@ -356,6 +356,9 @@ const createAbTestInputSchema = z.object({ }), }) .optional(), + enable_stratification: optionalBooleanSchema.describe( + "Enable recipient-domain stratification during holdout provisioning", + ), }); const analyzeAbTestInputSchema = testIdInputSchema.extend({ diff --git a/packages/abtest/src/types.ts b/packages/abtest/src/types.ts index 42ad2f7e..cf5932bb 100644 --- a/packages/abtest/src/types.ts +++ b/packages/abtest/src/types.ts @@ -214,6 +214,11 @@ export interface AbTestConfig { // provided unlocked, createTest locks it before provisioning so the // assignment manifest cannot be separated from a frozen hypothesis. hypothesis?: import("./hypothesis").HypothesisMetadata; + // Recipient-domain stratification policy. When enabled, the holdout + // provisioning path computes a stratified quota matrix from the audience + // and stores it on AbTest.stratification. Optional; defaults to the + // disabled policy. + stratificationPolicy?: import("./stratification").StratificationPolicyV1; } export interface AbTestInput { @@ -272,6 +277,11 @@ export interface CreateAbTestInput { exclusion_window_hours: number; }; }; + // Enable recipient-domain stratification during holdout provisioning. + // When true, the service applies the default stratification policy + // (gmail/naver/daum/kakao + other/unknown fallbacks) and stores the + // computed quota matrix on AbTest.stratification. + enable_stratification?: boolean; } export interface AnalyzeAbTestInput { From d3e9078f84d1a6cd1ccbbee9da453c6d621fddbb Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 13:50:02 +0900 Subject: [PATCH 11/20] feat(abtest): expose hypothesis+stratification through CLI, update root docs Addresses the @codex P1 findings on commit 48bf2c8: - buildCreateInputFromFlags now accepts --enable-stratification and --hypothesis (JSON), and the create command declares the corresponding options. The interactive prompt also asks about stratification and an optional hypothesis JSON document. CLI users can now enable both new behaviors on parity with MCP callers. - Add a Hypothesis pre-registration and recipient-domain stratification section to both root README.md and README_ko.md, with example commands pointing at the package README for full validation rules. --- README.md | 24 ++++++++++++++++ README_ko.md | 22 +++++++++++++++ apps/cli/src/commands/abtest.ts | 49 +++++++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+) diff --git a/README.md b/README.md index 85df7647..6f8bc054 100644 --- a/README.md +++ b/README.md @@ -465,6 +465,30 @@ send results. Summary of the current behavior: See [`packages/abtest/README.md`](packages/abtest/README.md) for the underlying Listmonk API behavior and spike rationale. +### Hypothesis pre-registration and recipient-domain stratification + +`abtest create` accepts two advanced experimentation inputs: + +- `--hypothesis '{...}'` — a pre-registration hypothesis (objective, primary + metric, expected lift, owner, experiment scope). The service locks it + (SHA-256 checksum) before recipient assignment so the metadata cannot + change after recipients are set. +- `--enable-stratification` — classify subscribers by email-domain provider + and compute a constrained quota matrix so each provider stratum gets a + proportional share of every variant/holdout group. + +```bash +listmonk-cli abtest create \ + --name "Subject Line Test" \ + --variants '[...]' --lists 1,2 \ + --enable-stratification \ + --hypothesis '{"objective":"Increase CTR","hypothesis":"Shorter subject lifts CTR","primary_metric":{"type":"click_rate","direction":"maximize"},"expected_lift":{"kind":"relative","value":0.1},"owner":{"id":"user-1"},"experiment_scope":{"channel":"email","experiment_family_key":"onboarding.welcome","attribution_window_hours":72,"exclusion_window_hours":168}}' +``` + +See [`packages/abtest/README.md`](packages/abtest/README.md) for the full +validation rules, the stratified quota solver, and bilingual (EN/KO) +guidance. + ## Ops Automation Commands ```bash diff --git a/README_ko.md b/README_ko.md index a6c6525b..d95fd66a 100644 --- a/README_ko.md +++ b/README_ko.md @@ -460,6 +460,28 @@ A/B 테스트 도메인은 발송 결과를 왜곡할 수 있는 여러 정확 자세한 Listmonk API 동작과 spike 근거는 [`packages/abtest/README.md`](packages/abtest/README.md)를 참고하세요. +### 가설 사전 등록 및 수신자 도메인 층화 + +`abtest create`는 고급 실험 입력 두 가지를 받습니다: + +- `--hypothesis '{...}'` — 사전 등록 가설(목표, 주요 지표, 기대 효과, + 담당자, 실험 범위). 서비스가 수신자 할당 전에 잠금(SHA-256 체크섬) + 처리하므로 수신자가 정해진 뒤에는 가설을 변경할 수 없습니다. +- `--enable-stratification` — 구독자를 이메일 도메인 제공자별로 분류하고 + 제약된 할당량 행렬을 계산하여 각 제공자 층(stratum)이 모든 변형/홀드아웃 + 그룹의 비례 배분을 받도록 합니다. + +```bash +listmonk-cli abtest create \ + --name "Subject Line Test" \ + --variants '[...]' --lists 1,2 \ + --enable-stratification \ + --hypothesis '{"objective":"CTR 향상","hypothesis":"짧은 제목이 CTR을 높인다","primary_metric":{"type":"click_rate","direction":"maximize"},"expected_lift":{"kind":"relative","value":0.1},"owner":{"id":"user-1"},"experiment_scope":{"channel":"email","experiment_family_key":"onboarding.welcome","attribution_window_hours":72,"exclusion_window_hours":168}}' +``` + +전체 검증 규칙, 층화 할당량 솔버, 한영(EN/KO) 가이드는 +[`packages/abtest/README.md`](packages/abtest/README.md)를 참고하세요. + ## 운영 자동화 명령 ```bash diff --git a/apps/cli/src/commands/abtest.ts b/apps/cli/src/commands/abtest.ts index 7941908c..64ad3732 100644 --- a/apps/cli/src/commands/abtest.ts +++ b/apps/cli/src/commands/abtest.ts @@ -166,6 +166,8 @@ export function buildCreateInputFromFlags(flags: { "test-group-percentage"?: number; "auto-deploy-winner": boolean; "ignore-sample-size-warnings": boolean; + "enable-stratification"?: boolean; + hypothesis?: string; }): CreateAbTestInput { const parsedVariants = parseJson(flags.variants, "variants"); if (!Array.isArray(parsedVariants)) { @@ -180,6 +182,17 @@ export function buildCreateInputFromFlags(flags: { const baseSubject = flags.subject?.trim() ?? ""; const baseBody = flags.body?.trim() ?? ""; + // The hypothesis is a JSON document matching the CreateAbTestInput + // hypothesis shape (objective, primary_metric, expected_lift, owner, + // experiment_scope). Parsed here so CLI/MCP share the same contract. + const hypothesis = + flags.hypothesis !== undefined + ? (parseJson( + flags.hypothesis, + "hypothesis", + ) ?? undefined) + : undefined; + return { name: flags.name, campaign_id: String(flags["campaign-id"]), @@ -196,6 +209,8 @@ export function buildCreateInputFromFlags(flags: { test_group_percentage: testGroupPercentage, auto_deploy_winner: flags["auto-deploy-winner"], ignore_sample_size_warnings: flags["ignore-sample-size-warnings"], + enable_stratification: flags["enable-stratification"] ?? undefined, + hypothesis, }; } @@ -476,6 +491,29 @@ async function promptInteractiveInput( throw new Error("Prompt cancelled by user"); } + const stratifyResult = await clack.confirm({ + message: + "Enable recipient-domain stratification during holdout provisioning?", + initialValue: false, + }); + if (clack.isCancel(stratifyResult)) { + clack.cancel("Cancelled"); + throw new Error("Prompt cancelled by user"); + } + + // Optional pre-registration hypothesis as a JSON document. Empty input + // skips it; the shared service locks whatever is provided. + const hypothesisResult = await clack.text({ + message: + "Pre-registration hypothesis JSON (leave empty to skip)?", + placeholder: '{"objective": "...", "primary_metric": {...}, ...}', + defaultValue: "", + }); + if (clack.isCancel(hypothesisResult)) { + clack.cancel("Cancelled"); + throw new Error("Prompt cancelled by user"); + } + const input = buildCreateInputFromFlags({ name: nameResult, "campaign-id": Number(campaignIdResult), @@ -487,6 +525,9 @@ async function promptInteractiveInput( "test-group-percentage": Number(testGroupResult), "auto-deploy-winner": autoDeployResult, "ignore-sample-size-warnings": ignoreWarningsResult, + "enable-stratification": stratifyResult, + hypothesis: + hypothesisResult.trim().length > 0 ? hypothesisResult.trim() : undefined, }); clack.note( @@ -638,6 +679,14 @@ export default defineGroup({ description: "Ignore sample-size warnings", }, ), + "enable-stratification": option(z.coerce.boolean().optional(), { + description: + "Enable recipient-domain stratification during holdout provisioning", + }), + hypothesis: option(z.string().optional(), { + description: + "Pre-registration hypothesis as JSON (objective, primary_metric, expected_lift, owner, experiment_scope)", + }), }, handler: async ({ flags, ...args }) => { try { From 750d9aa6ae95f2ec67f8daa79779bff79b3836d5 Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 14:03:41 +0900 Subject: [PATCH 12/20] fix(abtest): reject null hypothesis JSON, strict ISO at persistence boundary Addresses @codex findings on commit d3e9078: - CLI: --hypothesis now rejects non-object JSON (e.g. null) instead of silently dropping it, so the shared schema validates the input the same way MCP does. - Persistence: isStoredHypothesis now uses the strict ISO 8601 validator (exported from hypothesis.ts) for createdAt and lockedAt, rejecting overflowed dates like 2026-02-30 and localized formats like 01/02/03 that Date.parse silently accepts. Previously the lenient isValidTimestamp helper allowed them at the file boundary. - README/README_ko: add the required --campaign-id flag to both hypothesis + stratification example commands. The remaining P1 (drive decisions from pre-registered metric/direction) and the small-stratum fallback / stratification floor-bound edge cases are tracked as Change Set C/D work. --- README.md | 1 + README_ko.md | 1 + apps/cli/src/commands/abtest.ts | 24 +++++++++++++++++------- packages/abtest/src/hypothesis.ts | 3 ++- packages/abtest/src/persistence.ts | 10 +++------- 5 files changed, 24 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 6f8bc054..6746d6a3 100644 --- a/README.md +++ b/README.md @@ -480,6 +480,7 @@ underlying Listmonk API behavior and spike rationale. ```bash listmonk-cli abtest create \ --name "Subject Line Test" \ + --campaign-id 1 \ --variants '[...]' --lists 1,2 \ --enable-stratification \ --hypothesis '{"objective":"Increase CTR","hypothesis":"Shorter subject lifts CTR","primary_metric":{"type":"click_rate","direction":"maximize"},"expected_lift":{"kind":"relative","value":0.1},"owner":{"id":"user-1"},"experiment_scope":{"channel":"email","experiment_family_key":"onboarding.welcome","attribution_window_hours":72,"exclusion_window_hours":168}}' diff --git a/README_ko.md b/README_ko.md index d95fd66a..2900719a 100644 --- a/README_ko.md +++ b/README_ko.md @@ -474,6 +474,7 @@ A/B 테스트 도메인은 발송 결과를 왜곡할 수 있는 여러 정확 ```bash listmonk-cli abtest create \ --name "Subject Line Test" \ + --campaign-id 1 \ --variants '[...]' --lists 1,2 \ --enable-stratification \ --hypothesis '{"objective":"CTR 향상","hypothesis":"짧은 제목이 CTR을 높인다","primary_metric":{"type":"click_rate","direction":"maximize"},"expected_lift":{"kind":"relative","value":0.1},"owner":{"id":"user-1"},"experiment_scope":{"channel":"email","experiment_family_key":"onboarding.welcome","attribution_window_hours":72,"exclusion_window_hours":168}}' diff --git a/apps/cli/src/commands/abtest.ts b/apps/cli/src/commands/abtest.ts index 64ad3732..fbf5863b 100644 --- a/apps/cli/src/commands/abtest.ts +++ b/apps/cli/src/commands/abtest.ts @@ -185,13 +185,23 @@ export function buildCreateInputFromFlags(flags: { // The hypothesis is a JSON document matching the CreateAbTestInput // hypothesis shape (objective, primary_metric, expected_lift, owner, // experiment_scope). Parsed here so CLI/MCP share the same contract. - const hypothesis = - flags.hypothesis !== undefined - ? (parseJson( - flags.hypothesis, - "hypothesis", - ) ?? undefined) - : undefined; + // Reject non-object JSON (e.g. "null") so the shared schema validates it + // the same way MCP does, rather than silently dropping it. + let hypothesis: CreateAbTestInput["hypothesis"] | undefined; + if (flags.hypothesis !== undefined) { + const parsed = parseJson( + flags.hypothesis, + "hypothesis", + ); + if (parsed === null || typeof parsed !== "object" || Array.isArray( + parsed, + )) { + throw new Error( + "hypothesis must be a JSON object matching the pre-registration shape", + ); + } + hypothesis = parsed; + } return { name: flags.name, diff --git a/packages/abtest/src/hypothesis.ts b/packages/abtest/src/hypothesis.ts index 5c474a85..1604f96a 100644 --- a/packages/abtest/src/hypothesis.ts +++ b/packages/abtest/src/hypothesis.ts @@ -75,8 +75,9 @@ export class HypothesisValidationError extends Error { * accept, including the year-zero string "0", localized formats like * "01/02/03", and overflowed calendar dates like "2026-02-30". The date * components are reconstructed and compared so overflow rolls over are caught. + * Exported so the persistence boundary can reuse the same strict check. */ -function isStrictIsoTimestamp(value: unknown): boolean { +export function isStrictIsoTimestamp(value: unknown): boolean { if (typeof value !== "string") return false; const re = /^\d{4}-\d{2}-\d{2}([T ]\d{2}:\d{2}:\d{2}(\.\d{1,3})?(Z|[+-]\d{2}:\d{2})?)?$/; diff --git a/packages/abtest/src/persistence.ts b/packages/abtest/src/persistence.ts index c20f04a3..4c28f69d 100644 --- a/packages/abtest/src/persistence.ts +++ b/packages/abtest/src/persistence.ts @@ -11,7 +11,7 @@ import { import type { ListmonkClient } from "@listmonk-ops/openapi"; import { AbTestNotFoundError } from "./errors"; import { createAbTestExecutors, type AbTestExecutors } from "./factory"; -import { verifyHypothesisChecksum } from "./hypothesis"; +import { isStrictIsoTimestamp, verifyHypothesisChecksum } from "./hypothesis"; import type { AbTest } from "./types"; export { AbTestNotFoundError } from "./errors"; @@ -348,9 +348,7 @@ function isStoredHypothesis(value: unknown): boolean { if (typeof value.hypothesis !== "string" || value.hypothesis.trim() === "") { return false; } - if (typeof value.createdAt !== "string" || !isValidTimestamp( - value.createdAt, - )) { + if (!isStrictIsoTimestamp(value.createdAt)) { return false; } // primaryMetric @@ -438,9 +436,7 @@ function isStoredHypothesis(value: unknown): boolean { // 64-character hex string AND must cryptographically match the recomputed // canonical checksum, so tampered records are rejected at load time. if (value.lockedAt !== undefined) { - if ( - typeof value.lockedAt !== "string" || !isValidTimestamp(value.lockedAt) - ) { + if (!isStrictIsoTimestamp(value.lockedAt)) { return false; } if ( From 0e0ea0ed2b1f95375c06c00954eed29367546203 Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 14:22:24 +0900 Subject: [PATCH 13/20] fix(abtest): boolean parsing, small-stratum merge, quota search, docs Addresses CodeRabbit and @codex findings on commit 750d9aa: CodeRabbit: - --enable-stratification now parses "true"/"false" strings explicitly instead of relying on z.coerce.boolean() truthiness (which treated the string "false" as true). - basic.ts rejects an unknown expected_lift.kind instead of treating it as absolute, matching the discriminated union contract. - README/README_ko clarify that stratification computes and stores the quota matrix; applying it to assignment slices is deferred. - Interactive confirmation summary includes the stratification flag and a compact hypothesis summary. @codex P2: - Apply the configured small-stratum fallback: providers below minimumStratumSize are merged into "other" before solving. - Search all feasible (deficit, surplus) swap pairs each iteration instead of greedily fixing on the first pair, so the only viable swap for a later pair is not consumed. - Validate persisted stratification relationships: reject cells for unknown strata/groups, cells disagreeing with the quotas matrix, duplicate cells, and quota rows whose sum does not match stratumSizes. Verified against 8000 randomized stratification trials. --- README.md | 4 +- README_ko.md | 4 +- apps/cli/src/commands/abtest.ts | 26 ++++- packages/abtest/src/basic.ts | 16 ++- packages/abtest/src/listmonk-integration.ts | 21 ++++ packages/abtest/src/persistence.ts | 35 +++++- packages/abtest/src/stratification.ts | 112 +++++++++++--------- 7 files changed, 155 insertions(+), 63 deletions(-) diff --git a/README.md b/README.md index 6746d6a3..9201503f 100644 --- a/README.md +++ b/README.md @@ -475,7 +475,9 @@ underlying Listmonk API behavior and spike rationale. change after recipients are set. - `--enable-stratification` — classify subscribers by email-domain provider and compute a constrained quota matrix so each provider stratum gets a - proportional share of every variant/holdout group. + proportional share of every variant/holdout group. The quota matrix is + computed and stored on the test for reporting/validation; applying it to + the actual assignment slices is deferred to a follow-up change set. ```bash listmonk-cli abtest create \ diff --git a/README_ko.md b/README_ko.md index 2900719a..a9cff216 100644 --- a/README_ko.md +++ b/README_ko.md @@ -469,7 +469,9 @@ A/B 테스트 도메인은 발송 결과를 왜곡할 수 있는 여러 정확 처리하므로 수신자가 정해진 뒤에는 가설을 변경할 수 없습니다. - `--enable-stratification` — 구독자를 이메일 도메인 제공자별로 분류하고 제약된 할당량 행렬을 계산하여 각 제공자 층(stratum)이 모든 변형/홀드아웃 - 그룹의 비례 배분을 받도록 합니다. + 그룹의 비례 배분을 받도록 합니다. 할당량 행렬은 보고/검증을 위해 계산되어 + 테스트에 저장되며, 실제 할당 슬라이스에 적용하는 것은 후속 변경 세트로 + 연기됩니다. ```bash listmonk-cli abtest create \ diff --git a/apps/cli/src/commands/abtest.ts b/apps/cli/src/commands/abtest.ts index fbf5863b..21a8500b 100644 --- a/apps/cli/src/commands/abtest.ts +++ b/apps/cli/src/commands/abtest.ts @@ -553,6 +553,16 @@ async function promptInteractiveInput( testingMode: input.testing_mode, testGroupPercentage: input.test_group_percentage, autoDeployWinner: input.auto_deploy_winner, + enableStratification: input.enable_stratification ?? false, + hypothesis: input.hypothesis + ? { + objective: input.hypothesis.objective, + primaryMetric: input.hypothesis.primary_metric.type, + direction: input.hypothesis.primary_metric.direction, + familyKey: + input.hypothesis.experiment_scope.experiment_family_key, + } + : undefined, }, null, 2, @@ -689,10 +699,18 @@ export default defineGroup({ description: "Ignore sample-size warnings", }, ), - "enable-stratification": option(z.coerce.boolean().optional(), { - description: - "Enable recipient-domain stratification during holdout provisioning", - }), + "enable-stratification": option( + z + .string() + .optional() + .transform((v) => + v === undefined ? undefined : v === "true", + ), + { + description: + "Enable recipient-domain stratification during holdout provisioning", + }, + ), hypothesis: option(z.string().optional(), { description: "Pre-registration hypothesis as JSON (objective, primary_metric, expected_lift, owner, experiment_scope)", diff --git a/packages/abtest/src/basic.ts b/packages/abtest/src/basic.ts index 37be6ec7..31dfbdd4 100644 --- a/packages/abtest/src/basic.ts +++ b/packages/abtest/src/basic.ts @@ -63,11 +63,17 @@ export class CreateAbTestCommand { kind: "relative", value: input.hypothesis.expected_lift.value, } - : { - kind: "absolute", - value: input.hypothesis.expected_lift.value, - unit: input.hypothesis.expected_lift.unit, - }, + : input.hypothesis.expected_lift.kind === "absolute" + ? { + kind: "absolute", + value: input.hypothesis.expected_lift.value, + unit: input.hypothesis.expected_lift.unit, + } + : (() => { + throw new ValidationError( + `expected_lift.kind must be "relative" or "absolute", received ${JSON.stringify((input.hypothesis?.expected_lift as { kind?: unknown } | undefined)?.kind)}`, + ); + })(), owner: { id: input.hypothesis.owner.id, displayName: input.hypothesis.owner.display_name, diff --git a/packages/abtest/src/listmonk-integration.ts b/packages/abtest/src/listmonk-integration.ts index ce9c4889..9fa560e6 100644 --- a/packages/abtest/src/listmonk-integration.ts +++ b/packages/abtest/src/listmonk-integration.ts @@ -343,6 +343,27 @@ export class ListmonkAbTestIntegration { const stratum = classifier(member.email ?? ""); stratumSizes[stratum] = (stratumSizes[stratum] ?? 0) + 1; } + // Apply the configured small-stratum fallback: providers + // below minimumStratumSize are merged into "other" so the + // quota matrix matches the documented policy behavior. + if ( + stratificationPolicy.minimumStratumSize > 0 && + stratificationPolicy.smallStratumFallback === + "merge_into_other" + ) { + const otherKey = stratificationPolicy.otherStratumKey; + for (const [stratum, size] of Object.entries(stratumSizes)) { + if ( + stratum !== otherKey && + stratum !== stratificationPolicy.unknownStratumKey && + size < stratificationPolicy.minimumStratumSize + ) { + stratumSizes[otherKey] = + (stratumSizes[otherKey] ?? 0) + size; + delete stratumSizes[stratum]; + } + } + } // Build exact group counts from the manifest groups. const groupExactCounts: Record = {}; const groupOrder: string[] = []; diff --git a/packages/abtest/src/persistence.ts b/packages/abtest/src/persistence.ts index 4c28f69d..c85f5455 100644 --- a/packages/abtest/src/persistence.ts +++ b/packages/abtest/src/persistence.ts @@ -300,7 +300,9 @@ function isStoredStratification(value: unknown): boolean { return false; } } - // Each cell must have the required shape with non-negative values. + // Each cell must have the required shape with non-negative values, and + // must reference a known stratum/group and agree with the quotas matrix. + const seenCells = new Set(); for (const cell of cells) { if ( !isRecord(cell) || @@ -316,6 +318,37 @@ function isStoredStratification(value: unknown): boolean { ) { return false; } + // Reject cells referencing unknown strata/groups. + if ( + !(cell.stratumKey in quotas) || + !(cell.stratumKey in stratumSizes) + ) { + return false; + } + const row = quotas[cell.stratumKey]; + if (!isRecord(row) || !(cell.groupKey in row)) { + return false; + } + // Reject cells whose quota disagrees with the quotas matrix. + if (row[cell.groupKey] !== cell.quota) { + return false; + } + // Reject duplicate cells. + const cellKey = `${cell.stratumKey}:${cell.groupKey}`; + if (seenCells.has(cellKey)) return false; + seenCells.add(cellKey); + } + // Every quota row must sum to its stratum size. + for (const [sk, row] of Object.entries(quotas)) { + if (!isRecord(row)) return false; + const rowSum = Object.values(row).reduce( + (sum, n) => sum + (typeof n === "number" ? n : 0), + 0, + ); + const expected = stratumSizes[sk]; + if (typeof expected !== "number" || rowSum !== expected) { + return false; + } } return true; } diff --git a/packages/abtest/src/stratification.ts b/packages/abtest/src/stratification.ts index 08e56c40..737650b6 100644 --- a/packages/abtest/src/stratification.ts +++ b/packages/abtest/src/stratification.ts @@ -295,62 +295,72 @@ export function computeStratifiedQuotas(params: { const maxIterations = stratumKeys.length * groupOrder.length * groupOrder.length + 16; for (let iter = 0; iter < maxIterations; iter += 1) { - const deficitGroup = groupOrder.find((g) => (columnDeficit[g] ?? 0) > 0); - const surplusGroup = groupOrder.find((g) => (columnDeficit[g] ?? 0) < 0); - if (!deficitGroup || !surplusGroup) break; + const deficitGroups = groupOrder.filter((g) => (columnDeficit[g] ?? 0) > 0); + const surplusGroups = groupOrder.filter((g) => (columnDeficit[g] ?? 0) < 0); + if (deficitGroups.length === 0 || surplusGroups.length === 0) break; - // Choose the row where the deficit cell is most below ideal and the - // surplus cell can donate. We prefer swaps that keep cells within their - // floor-or-ceiling allocation: the donor stays at/above its floor and - // the receiver stays at/below its ceiling. When no such bounded swap is - // possible (column totals may require a cell outside the naive floor/ - // ceiling band), fall back to the row that minimizes how far outside - // the band the resulting cells would land. - let bestStratum: string | null = null; - let bestScore = -Infinity; - let bestBounded = false; - for (const sk of stratumKeys) { - const row = quotas[sk]; - if (!row) continue; - const surplusQuota = row[surplusGroup] ?? 0; - const deficitQuota = row[deficitGroup] ?? 0; - const surplusIdeal = idealLookup.get(`${sk}:${surplusGroup}`) ?? 0; - const deficitIdeal = idealLookup.get(`${sk}:${deficitGroup}`) ?? 0; - const surplusBounded = surplusQuota > Math.floor(surplusIdeal); - const deficitBounded = deficitQuota < Math.ceil(deficitIdeal); - const bounded = surplusBounded && deficitBounded; - // Always require a positive donor and a receiver below ceiling so - // the swap is physically valid (no negative quota, no receiver - // already at ceiling that the swap would exceed). - if (surplusQuota <= 0) continue; - if (deficitQuota >= Math.ceil(deficitIdeal)) continue; - const deficitDev = deficitQuota - deficitIdeal; - const surplusDev = surplusQuota - surplusIdeal; - const score = surplusDev - deficitDev; - // Prefer bounded swaps; among the same boundedness, prefer the - // best score. - if (bestStratum === null) { - bestStratum = sk; - bestScore = score; - bestBounded = bounded; - } else if (bounded && !bestBounded) { - // A bounded swap beats an unbounded incumbent. - bestStratum = sk; - bestScore = score; - bestBounded = bounded; - } else if (bounded === bestBounded && score > bestScore) { - bestStratum = sk; - bestScore = score; + // Evaluate all feasible (deficit, surplus, stratum) triples and pick + // the best swap rather than greedily fixing on the first deficit and + // first surplus group, which could consume the only viable swap for a + // later pair. + let bestSwap: { + deficitGroup: string; + surplusGroup: string; + stratum: string; + bounded: boolean; + score: number; + } | null = null; + for (const deficitGroup of deficitGroups) { + for (const surplusGroup of surplusGroups) { + // Choose the best row for this (deficit, surplus) pair. Prefer + // swaps that keep cells within their floor-or-ceiling allocation. + for (const sk of stratumKeys) { + const row = quotas[sk]; + if (!row) continue; + const surplusQuota = row[surplusGroup] ?? 0; + const deficitQuota = row[deficitGroup] ?? 0; + const surplusIdeal = + idealLookup.get(`${sk}:${surplusGroup}`) ?? 0; + const deficitIdeal = + idealLookup.get(`${sk}:${deficitGroup}`) ?? 0; + // Require a positive donor and a receiver below ceiling. + if (surplusQuota <= 0) continue; + if (deficitQuota >= Math.ceil(deficitIdeal)) continue; + const surplusBounded = surplusQuota > Math.floor(surplusIdeal); + const deficitBounded = deficitQuota < Math.ceil(deficitIdeal); + const bounded = surplusBounded && deficitBounded; + const deficitDev = deficitQuota - deficitIdeal; + const surplusDev = surplusQuota - surplusIdeal; + const score = surplusDev - deficitDev; + // Pick the globally best swap across all pairs. + if ( + bestSwap === null || + (bounded && !bestSwap.bounded) || + (bounded === bestSwap.bounded && score > bestSwap.score) + ) { + bestSwap = { + deficitGroup, + surplusGroup, + stratum: sk, + bounded, + score, + }; + } + } } } - if (!bestStratum) break; + if (!bestSwap) break; - const row = quotas[bestStratum]; + const row = quotas[bestSwap.stratum]; if (!row) break; - row[deficitGroup] = (row[deficitGroup] ?? 0) + 1; - row[surplusGroup] = (row[surplusGroup] ?? 0) - 1; - columnDeficit[deficitGroup] = (columnDeficit[deficitGroup] ?? 0) - 1; - columnDeficit[surplusGroup] = (columnDeficit[surplusGroup] ?? 0) + 1; + row[bestSwap.deficitGroup] = + (row[bestSwap.deficitGroup] ?? 0) + 1; + row[bestSwap.surplusGroup] = + (row[bestSwap.surplusGroup] ?? 0) - 1; + columnDeficit[bestSwap.deficitGroup] = + (columnDeficit[bestSwap.deficitGroup] ?? 0) - 1; + columnDeficit[bestSwap.surplusGroup] = + (columnDeficit[bestSwap.surplusGroup] ?? 0) + 1; } // Verify convergence: every column deficit should be resolved to zero. From 3865d842143ca777d28d6e677630311f165520d0 Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 14:36:15 +0900 Subject: [PATCH 14/20] fix(abtest): bare flag, lockedAt validation, unknown merge, cell coverage Addresses @codex P2 findings on commit 0e0ea0e: - --enable-stratification is a bare boolean flag (z.coerce.boolean().default false) matching the README example and existing flag conventions. - createTest validates a pre-locked hypothesis lockedAt as strict ISO 8601 before accepting it, rejecting garbage timestamps. - Small-stratum fallback now merges the unknown stratum into other too (previously only provider strata were merged). - Persisted stratification now requires a cell for every quota matrix entry, rejecting truncated cells arrays. --- apps/cli/src/commands/abtest.ts | 16 +++------- packages/abtest/src/abtest-service.ts | 34 +++++++++++++-------- packages/abtest/src/listmonk-integration.ts | 12 +++----- packages/abtest/src/persistence.ts | 7 +++++ 4 files changed, 36 insertions(+), 33 deletions(-) diff --git a/apps/cli/src/commands/abtest.ts b/apps/cli/src/commands/abtest.ts index 21a8500b..4bbb3bc2 100644 --- a/apps/cli/src/commands/abtest.ts +++ b/apps/cli/src/commands/abtest.ts @@ -699,18 +699,10 @@ export default defineGroup({ description: "Ignore sample-size warnings", }, ), - "enable-stratification": option( - z - .string() - .optional() - .transform((v) => - v === undefined ? undefined : v === "true", - ), - { - description: - "Enable recipient-domain stratification during holdout provisioning", - }, - ), + "enable-stratification": option(z.coerce.boolean().default(false), { + description: + "Enable recipient-domain stratification during holdout provisioning", + }), hypothesis: option(z.string().optional(), { description: "Pre-registration hypothesis as JSON (objective, primary_metric, expected_lift, owner, experiment_scope)", diff --git a/packages/abtest/src/abtest-service.ts b/packages/abtest/src/abtest-service.ts index d74d0897..b337aceb 100644 --- a/packages/abtest/src/abtest-service.ts +++ b/packages/abtest/src/abtest-service.ts @@ -12,6 +12,7 @@ import { fixedHorizonGate, } from "./statistics"; import { + isStrictIsoTimestamp, lockHypothesis, validateHypothesisMetadata, verifyHypothesisChecksum, @@ -188,19 +189,26 @@ export class AbTestService { // When a caller supplies an already-locked hypothesis, validate it // strictly and verify its checksum before accepting it, so tampered // or malformed metadata cannot reach remote campaign/list provisioning. - hypothesis: config.hypothesis - ? config.hypothesis.lockedAt - ? (() => { - validateHypothesisMetadata(config.hypothesis!, true); - if (!verifyHypothesisChecksum(config.hypothesis!)) { - throw new Error( - "Pre-locked hypothesis checksum verification failed; the metadata may have been tampered with", - ); - } - return config.hypothesis!; - })() - : lockHypothesis(config.hypothesis) - : undefined, + hypothesis: config.hypothesis + ? config.hypothesis.lockedAt + ? (() => { + validateHypothesisMetadata(config.hypothesis!, true); + if ( + !isStrictIsoTimestamp(config.hypothesis!.lockedAt) + ) { + throw new Error( + "Pre-locked hypothesis lockedAt is not a valid ISO 8601 timestamp", + ); + } + if (!verifyHypothesisChecksum(config.hypothesis!)) { + throw new Error( + "Pre-locked hypothesis checksum verification failed; the metadata may have been tampered with", + ); + } + return config.hypothesis!; + })() + : lockHypothesis(config.hypothesis) + : undefined, }; // Create Listmonk campaigns if integration is available diff --git a/packages/abtest/src/listmonk-integration.ts b/packages/abtest/src/listmonk-integration.ts index 9fa560e6..eceb387e 100644 --- a/packages/abtest/src/listmonk-integration.ts +++ b/packages/abtest/src/listmonk-integration.ts @@ -343,9 +343,9 @@ export class ListmonkAbTestIntegration { const stratum = classifier(member.email ?? ""); stratumSizes[stratum] = (stratumSizes[stratum] ?? 0) + 1; } - // Apply the configured small-stratum fallback: providers - // below minimumStratumSize are merged into "other" so the - // quota matrix matches the documented policy behavior. + // Apply the configured small-stratum fallback: any stratum + // (including unknown) below minimumStratumSize is merged into + // "other" so the quota matrix matches the documented policy. if ( stratificationPolicy.minimumStratumSize > 0 && stratificationPolicy.smallStratumFallback === @@ -353,11 +353,7 @@ export class ListmonkAbTestIntegration { ) { const otherKey = stratificationPolicy.otherStratumKey; for (const [stratum, size] of Object.entries(stratumSizes)) { - if ( - stratum !== otherKey && - stratum !== stratificationPolicy.unknownStratumKey && - size < stratificationPolicy.minimumStratumSize - ) { + if (stratum !== otherKey && size < stratificationPolicy.minimumStratumSize) { stratumSizes[otherKey] = (stratumSizes[otherKey] ?? 0) + size; delete stratumSizes[stratum]; diff --git a/packages/abtest/src/persistence.ts b/packages/abtest/src/persistence.ts index c85f5455..25dd77af 100644 --- a/packages/abtest/src/persistence.ts +++ b/packages/abtest/src/persistence.ts @@ -350,6 +350,13 @@ function isStoredStratification(value: unknown): boolean { return false; } } + // Every quota matrix entry must have a matching cell (no truncated cells). + for (const [sk, row] of Object.entries(quotas)) { + if (!isRecord(row)) return false; + for (const gk of Object.keys(row)) { + if (!seenCells.has(`${sk}:${gk}`)) return false; + } + } return true; } From d0fba9203edab02c43c18c32aefd07bb571a9114 Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 14:52:40 +0900 Subject: [PATCH 15/20] fix(abtest): stratum row coverage, pre-summary hypothesis validation Addresses @codex P2 findings on commit 3865d84: - Persisted stratification now requires a quota row for every stratum in stratumSizes, rejecting records where a stratum size exists without a matching quota row. - Interactive CLI flow validates the hypothesis shape (strict) before rendering the confirmation summary, so malformed input fails early with a clear error. --- apps/cli/src/commands/abtest.ts | 39 ++++++++++++++++++++++++++++++ packages/abtest/src/persistence.ts | 4 +++ 2 files changed, 43 insertions(+) diff --git a/apps/cli/src/commands/abtest.ts b/apps/cli/src/commands/abtest.ts index 4bbb3bc2..7e98e328 100644 --- a/apps/cli/src/commands/abtest.ts +++ b/apps/cli/src/commands/abtest.ts @@ -26,6 +26,7 @@ import { invokeRunAbTestOperation, invokeStopAbTestOperation, invokeTickAbTestsOperation, + validateHypothesisMetadata, validateStoredAbTestStore, } from "@listmonk-ops/abtest"; import { OutputUtils } from "@listmonk-ops/common"; @@ -540,6 +541,44 @@ async function promptInteractiveInput( hypothesisResult.trim().length > 0 ? hypothesisResult.trim() : undefined, }); + // Validate the hypothesis shape before showing the summary so malformed + // input fails early with a clear error rather than after confirmation. + if (input.hypothesis) { + validateHypothesisMetadata( + { + objective: input.hypothesis.objective, + hypothesis: input.hypothesis.hypothesis, + primaryMetric: { + type: input.hypothesis.primary_metric.type, + direction: input.hypothesis.primary_metric.direction, + }, + expectedLift: + input.hypothesis.expected_lift.kind === "relative" + ? { + kind: "relative", + value: input.hypothesis.expected_lift.value, + } + : { + kind: "absolute", + value: input.hypothesis.expected_lift.value, + unit: input.hypothesis.expected_lift.unit, + }, + owner: { id: input.hypothesis.owner.id }, + experimentScope: { + channel: input.hypothesis.experiment_scope.channel, + experimentFamilyKey: + input.hypothesis.experiment_scope.experiment_family_key, + attributionWindowHours: + input.hypothesis.experiment_scope.attribution_window_hours, + exclusionWindowHours: + input.hypothesis.experiment_scope.exclusion_window_hours, + }, + createdAt: new Date().toISOString(), + }, + true, + ); + } + clack.note( JSON.stringify( { diff --git a/packages/abtest/src/persistence.ts b/packages/abtest/src/persistence.ts index 25dd77af..d037650b 100644 --- a/packages/abtest/src/persistence.ts +++ b/packages/abtest/src/persistence.ts @@ -350,6 +350,10 @@ function isStoredStratification(value: unknown): boolean { return false; } } + // Every stratum in stratumSizes must have a corresponding quota row. + for (const sk of Object.keys(stratumSizes)) { + if (!(sk in quotas)) return false; + } // Every quota matrix entry must have a matching cell (no truncated cells). for (const [sk, row] of Object.entries(quotas)) { if (!isRecord(row)) return false; From 9788e089ca7341756e4fe51c1df77d1f76f8dcc4 Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 15:08:25 +0900 Subject: [PATCH 16/20] fix(abtest): require consistent group keys across quota rows Addresses @codex P2 finding on commit d0fba92: persisted stratification now requires every quota row to cover the same set of group keys, rejecting records where rows disagree on which groups exist. --- packages/abtest/src/persistence.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/abtest/src/persistence.ts b/packages/abtest/src/persistence.ts index d037650b..a84b08e4 100644 --- a/packages/abtest/src/persistence.ts +++ b/packages/abtest/src/persistence.ts @@ -275,9 +275,19 @@ function isStoredStratification(value: unknown): boolean { if (!isRecord(quotas) || !Array.isArray(cells) || !isRecord(stratumSizes)) { return false; } - // Every quota row must map group keys to non-negative finite numbers. - for (const row of Object.values(quotas)) { + // Every quota row must map group keys to non-negative finite numbers, and + // every row must cover the same set of group keys. + const quotaRows = Object.values(quotas); + const referenceGroupKeys = quotaRows.length > 0 + ? new Set(Object.keys(quotaRows[0] ?? {})) + : new Set(); + for (const row of quotaRows) { if (!isRecord(row)) return false; + const groupKeys = new Set(Object.keys(row)); + if (groupKeys.size !== referenceGroupKeys.size) return false; + for (const gk of groupKeys) { + if (!referenceGroupKeys.has(gk)) return false; + } for (const n of Object.values(row)) { if ( typeof n !== "number" || From 26b2936a6307a0d0ce2d169b35ac33b45454ae96 Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 15:31:38 +0900 Subject: [PATCH 17/20] fix(abtest): empty-email guard, consistent totalAudience, family-key docs Addresses CodeRabbit findings: - Empty-string emails no longer satisfy the all-members-have-email precondition; only non-empty trimmed emails qualify. - totalAudience for the quota solver now uses resolvedMembers.length (matching the stratum tally) instead of the snapshot subscriber count, avoiding a divergence that would violate the solver invariant. - The silent catch now logs the invariant violation before falling back, so failures are diagnosable. - README and error message clarify that family-key separators are [._-], not just dots. --- packages/abtest/README.md | 6 ++++-- packages/abtest/src/hypothesis.ts | 2 +- packages/abtest/src/listmonk-integration.ts | 21 ++++++++++++++++----- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/packages/abtest/README.md b/packages/abtest/README.md index 138bcfae..b65af039 100644 --- a/packages/abtest/README.md +++ b/packages/abtest/README.md @@ -742,8 +742,10 @@ const test = await abTestExecutors.createAbTest({ - Metric/unit coupling: `revenue_per_recipient` requires `currency_per_recipient` absolute lift; `click_rate`/`conversion_rate` require `percentage_point`. Relative lift is unit-agnostic. -- `experimentScope.experimentFamilyKey` must be dotted alphanumeric segments - (`onboarding.activation.day1`); `.`, `foo.`, and `foo..bar` are rejected. +- `experimentScope.experimentFamilyKey` must be lowercase alphanumeric segments + joined by single `.` / `_` / `-` separators (e.g. + `onboarding.activation.day1`, `cart-recovery_24h`); `.`, `foo.`, and + `foo..bar` are rejected. ### 가설 사전 등록 (Korean) diff --git a/packages/abtest/src/hypothesis.ts b/packages/abtest/src/hypothesis.ts index 1604f96a..1c590fa0 100644 --- a/packages/abtest/src/hypothesis.ts +++ b/packages/abtest/src/hypothesis.ts @@ -274,7 +274,7 @@ export function validateHypothesisMetadata( // separators from [._-]. if (!/^[a-z0-9]+(?:[._-][a-z0-9]+)*$/.test(scope.experimentFamilyKey)) { throw new HypothesisValidationError( - `experimentFamilyKey must be dotted alphanumeric segments (e.g. "onboarding.activation"), received "${scope.experimentFamilyKey}"`, + `experimentFamilyKey must be lowercase alphanumeric segments joined by [._-] separators (e.g. "onboarding.activation"), received "${scope.experimentFamilyKey}"`, ); } if ( diff --git a/packages/abtest/src/listmonk-integration.ts b/packages/abtest/src/listmonk-integration.ts index eceb387e..f3d64268 100644 --- a/packages/abtest/src/listmonk-integration.ts +++ b/packages/abtest/src/listmonk-integration.ts @@ -331,8 +331,12 @@ export class ListmonkAbTestIntegration { // bucket email-less subscribers into "unknown" and skew the // proportions. const allMembersHaveEmail = - resolvedMembers.length > 0 && - resolvedMembers.every((member) => member.email !== undefined); + resolvedMembers.length > 0 && + resolvedMembers.every( + (member) => + typeof member.email === "string" && + member.email.trim().length > 0, + ); if (allMembersHaveEmail) { try { // Build the provider-domain lookup once and classify each @@ -375,10 +379,17 @@ export class ListmonkAbTestIntegration { stratumSizes, groupExactCounts, groupOrder, - totalAudience: resolvedSnapshot.subscriberCount, + // Use resolvedMembers.length, not the snapshot count, + // so the divisor matches the stratum tally exactly. + totalAudience: resolvedMembers.length, }); - } catch { - // Stratification is non-critical; leave it undefined so + } catch (stratificationError) { + // Log the invariant violation instead of silently swallowing + // it, then fall back to an undefined stratification so + // provisioning proceeds with the manifest assignment. + console.warn( + `Stratification computation failed: ${(stratificationError as Error).message}`, + ); // provisioning proceeds with the manifest assignment. stratification = undefined; } From 433ef741244d27e066df6ee94d5a11a9ae651c8b Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 15:48:35 +0900 Subject: [PATCH 18/20] fix(abtest): owner.displayName validation, docs accuracy Addresses @codex/CodeRabbit findings on commit 26b2936: - validateHypothesisMetadata now checks owner.displayName is a string when present, rejecting untyped callers that pass a non-string. - Package README clarifies that stratification computes/stores the quota matrix and assignment application is a planned follow-up (the claim that it prevents provider dominance overstates current behavior). - Korean guide lists all accepted family-key separators ([._-]). --- packages/abtest/README.md | 9 ++++++--- packages/abtest/src/hypothesis.ts | 8 ++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/abtest/README.md b/packages/abtest/README.md index b65af039..77857a31 100644 --- a/packages/abtest/README.md +++ b/packages/abtest/README.md @@ -760,14 +760,17 @@ A/B 테스트에 **사전 등록된 가설**을 설정하면 수신자 할당 `experimentScope`, `createdAt`)를 재귀적으로 정규화하므로 잠금 후 어떤 변경도 무효화됩니다. - `createdAt`/`lockedAt`은 엄격한 ISO 8601이어야 합니다. -- `experimentFamilyKey`는 점으로 구분된 영숫자 세그먼트여야 합니다. +- `experimentFamilyKey`는 `.` / `_` / `-` 로 구분된 소문자 영숫자 세그먼트여야 합니다. ## Recipient-domain stratification (advanced experimentation) Stratification classifies subscribers by email-domain provider and computes a **constrained quota matrix** so each provider stratum gets a proportional share -of every variant/holdout group. This prevents a single large provider (e.g. -Gmail) from dominating one variant and skewing results. +of every variant/holdout group. The quota matrix is computed and stored for +reporting and validation. Note: applying these quotas to the actual recipient +assignment slices is a planned follow-up; today the assignment itself remains +the deterministic largest-remainder manifest, and the quota matrix documents +the target proportional allocation. ```typescript import { diff --git a/packages/abtest/src/hypothesis.ts b/packages/abtest/src/hypothesis.ts index 1c590fa0..c904a22f 100644 --- a/packages/abtest/src/hypothesis.ts +++ b/packages/abtest/src/hypothesis.ts @@ -246,6 +246,14 @@ export function validateHypothesisMetadata( "owner.id must be a non-empty string", ); } + if ( + owner.displayName !== undefined && + typeof owner.displayName !== "string" + ) { + throw new HypothesisValidationError( + `owner.displayName must be a string when present, received ${JSON.stringify(owner.displayName)}`, + ); + } } if (metadata.experimentScope !== undefined) { const scope = metadata.experimentScope; From 8b61ea5e5f36f0b9b71dd3b5cdbbbd4a41defcbb Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 16:05:09 +0900 Subject: [PATCH 19/20] fix(abtest): require manifest for manifest_v1 provenance Addresses @codex P2 finding on commit 433ef74: a persisted record with assignmentProvenance "manifest_v1" must also carry an assignmentManifest. Without this, a corrupted record could claim manifest-based provenance while having no manifest. --- packages/abtest/src/persistence.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/abtest/src/persistence.ts b/packages/abtest/src/persistence.ts index a84b08e4..56036b16 100644 --- a/packages/abtest/src/persistence.ts +++ b/packages/abtest/src/persistence.ts @@ -240,6 +240,9 @@ function isStoredAbTest(value: unknown): boolean { (value.assignmentProvenance === undefined || value.assignmentProvenance === "manifest_v1" || value.assignmentProvenance === "legacy_unavailable") && + // manifest_v1 provenance requires an actual assignment manifest. + (value.assignmentProvenance !== "manifest_v1" || + value.assignmentManifest !== undefined) && // Pre-registration hypothesis: optional, but the nested shape and the // locked-state checksum invariant are validated when present so that // loadStoredAbTests never hydrates malformed or tampered metadata. From 2097d248c2c7ed5966e7ea183482ea53d1e2a491 Mon Sep 17 00:00:00 2001 From: imjlk Date: Sat, 25 Jul 2026 16:19:26 +0900 Subject: [PATCH 20/20] docs(abtest): clarify Korean stratification does not change assignments Addresses @codex P2 finding: the Korean stratification guide now states that quota matrices are computed/stored for reporting and that applying them to assignment slices is a planned follow-up, matching the English section. --- packages/abtest/README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/abtest/README.md b/packages/abtest/README.md index 77857a31..c090d2bd 100644 --- a/packages/abtest/README.md +++ b/packages/abtest/README.md @@ -804,8 +804,10 @@ the `AbTest.stratification` field for reporting and validation. 층화는 구독자를 이메일 도메인 제공자별로 분류하고, 각 제공자 층(stratum)이 모든 변형/홀드아웃 그룹의 비례 배분을 받도록 **제약된 할당량 행렬**을 -계산합니다. 단일 대형 제공자(예: Gmail)가 하나의 변형을 독점하여 결과를 -왜곡하는 것을 방지합니다. +계산합니다. 할당량 행렬은 보고/검증을 위해 계산되어 저장됩니다. 참고: +이 할당량을 실제 수신자 할당 슬라이스에 적용하는 것은 후속 작업이며, +현재 할당 자체는 결정론적 largest-remainder 매니페스트를 그대로 사용하고 +할당량 행렬은 목표 비례 배분을 문서화합니다. - `classifyStratum`으로 구독자를 분류하고, `computeStratifiedQuotas`로 할당량 행렬을 계산합니다.