Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
99e59e9
feat(abtest): add hypothesis metadata for pre-registration
imjlk Jul 25, 2026
42ecc34
feat(abtest): add recipient-domain stratification for assignment
imjlk Jul 25, 2026
39dafac
fix(abtest): validate totalAudience and assert quota convergence
imjlk Jul 25, 2026
d0a34c3
fix(abtest): harden hypothesis pre-registration integrity
imjlk Jul 25, 2026
60ef214
fix(abtest): verify locked hypothesis checksum at load time
imjlk Jul 25, 2026
461952d
fix(abtest): wire hypothesis+stratification into production paths
imjlk Jul 25, 2026
e79acec
fix(abtest): isolate stratification from provisioning failure path
imjlk Jul 25, 2026
853d6bf
fix(abtest): preserve legacy manifests, harden stratification validation
imjlk Jul 25, 2026
54d895b
fix(abtest): export classifier, validate quota components, verify pre…
imjlk Jul 25, 2026
48bf2c8
fix(abtest): forward stratification policy from create flows
imjlk Jul 25, 2026
d3e9078
feat(abtest): expose hypothesis+stratification through CLI, update ro…
imjlk Jul 25, 2026
750d9aa
fix(abtest): reject null hypothesis JSON, strict ISO at persistence b…
imjlk Jul 25, 2026
0e0ea0e
fix(abtest): boolean parsing, small-stratum merge, quota search, docs
imjlk Jul 25, 2026
3865d84
fix(abtest): bare flag, lockedAt validation, unknown merge, cell cove…
imjlk Jul 25, 2026
d0fba92
fix(abtest): stratum row coverage, pre-summary hypothesis validation
imjlk Jul 25, 2026
9788e08
fix(abtest): require consistent group keys across quota rows
imjlk Jul 25, 2026
26b2936
fix(abtest): empty-email guard, consistent totalAudience, family-key …
imjlk Jul 25, 2026
433ef74
fix(abtest): owner.displayName validation, docs accuracy
imjlk Jul 25, 2026
8b61ea5
fix(abtest): require manifest for manifest_v1 provenance
imjlk Jul 25, 2026
2097d24
docs(abtest): clarify Korean stratification does not change assignments
imjlk Jul 25, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .sampo/changesets/abtest-hypothesis.md
Original file line number Diff line number Diff line change
@@ -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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Document the new public experimentation APIs bilingually

The changesets publish hypothesis locking and recipient-domain stratification as minor user-facing additions, but neither README.md nor README_ko.md documents their contracts, validation rules, or usage, and the package README is unchanged as well. Add matching English and Korean guidance so operators and library consumers can discover and correctly use the newly exported behavior.

AGENTS.md reference: AGENTS.md:L233-L237

Useful? React with 👍 / 👎.

210 changes: 210 additions & 0 deletions packages/abtest/src/hypothesis.ts
Original file line number Diff line number Diff line change
@@ -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";
Comment on lines +52 to +54

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Analyze the pre-registered primary metric

When a hypothesis pre-registers conversion_rate, revenue_per_recipient, or a minimize direction, analyzeTest never reads these fields: it dynamically chooses conversion rate only after observing any conversions, otherwise chooses click rate, and always selects the maximum value. A click-rate difference can therefore be declared significant and deployed for a conversion hypothesis, a minimizing test selects the worst variant, and a revenue hypothesis cannot drive a decision at all. Use the locked primary metric and direction for both significance testing and winner selection.

Useful? React with 👍 / 👎.

Comment on lines +52 to +54

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Report the pre-registered primary metric

When a test supplies this field, buildExperimentReport() still derives primaryMetric solely from whether any result has conversions, ignoring the locked hypothesis. Thus a click-rate hypothesis with observed conversions is reported as conversion_rate, and a revenue_per_recipient hypothesis can never be reported correctly, defeating the stated stable reference for experiment reports. Prefer test.hypothesis.primaryMetric.type when present and retain the existing inference only for legacy tests.

Useful? React with 👍 / 👎.

};
expectedLift: ExpectedLift;
Comment on lines +52 to +56

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Couple absolute-lift units to the primary metric

The independent types permit semantically incompatible metadata such as primaryMetric.type: "click_rate" with an absolute currency_per_recipient lift, or revenue_per_recipient with percentage_point; both combinations also pass validation and can be locked. Model or validate the metric/unit pairing so the pre-registered lift has an interpretable meaning for later analysis and reporting.

Useful? React with 👍 / 👎.

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<HypothesisMetadata>,
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}`,
);
}
Comment on lines +178 to +235

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reject malformed nested launch metadata

Strict validation only requires the nested objects to be present and checks expectedLift.value; it never validates primaryMetric.type/direction, expectedLift.kind or the required absolute-lift unit, and it does not require or validate createdAt. Thus runtime input such as { primaryMetric: {}, expectedLift: { kind: "bogus", value: 1 } } without createdAt passes strict validation and can be locked as a purportedly valid pre-registration.

Useful? React with 👍 / 👎.

}
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",
);
}
Comment on lines +244 to +248

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Validate optional owner display names before locking

For JavaScript or otherwise untyped callers, an owner such as { id: "user-1", displayName: 42 } passes strict validation and lockHypothesis produces a checksum, even though isStoredHypothesis later rejects the same record because displayName is not a string. The exported runtime validator can therefore create locked metadata that the package's persistence boundary cannot hydrate; validate the optional display name in this owner block before locking.

Useful? React with 👍 / 👎.

}
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}"`,
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require non-empty family-key segments

The character-class check accepts delimiter-only and empty-segment keys such as ., -, .foo, foo., and foo..bar. These do not form the documented dotted family identifier and allow a separator typo to create a different collision namespace rather than being rejected; require an alphanumeric segment at each end and between separators.

Useful? React with 👍 / 👎.

}
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}`,
);
}
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/**
* 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());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve nested fields in the hypothesis checksum

JSON.stringify applies this array replacer recursively, so nested keys such as type, direction, kind, value, id, and experimentFamilyKey are omitted and each nested object is serialized as {}. Consequently, changing the primary metric, expected lift, owner, or experiment scope after locking still makes verifyHypothesisChecksum return true, defeating the pre-registration integrity guarantee; use deterministic recursive canonicalization that retains nested fields.

Useful? React with 👍 / 👎.

return createHash("sha256").update(json, "utf8").digest("hex");
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}

/**
* 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) {
Comment on lines +366 to +368

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Validate the supplied lock timestamp before locking

When a caller uses the supported timestamp override with lockHypothesis(metadata, ""), the function returns a checksum with an empty lockedAt; verifyHypothesisChecksum then returns false and a subsequent call is allowed because the double-lock guard tests timestamp truthiness. Any other non-ISO string is also accepted as a valid lock timestamp, so validate the override before creating the locked object.

Useful? React with 👍 / 👎.

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 };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Detach locked metadata from the caller's nested objects

Because this is only a shallow copy, the returned locked hypothesis still shares primaryMetric, expectedLift, owner, and experimentScope with the caller's draft object. If a library caller reuses or edits that draft after locking—for example, changing draft.primaryMetric.type for another experiment—the supposedly locked object changes too and its checksum becomes invalid, which can corrupt the in-memory test and make the subsequently written store unloadable. Deep-clone or freeze the nested metadata when creating the locked snapshot.

Useful? React with 👍 / 👎.

Comment on lines +381 to +382

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bind the lock timestamp into the hypothesis checksum

When a locked record's lockedAt is changed to any other valid timestamp, verifyHypothesisChecksum() still returns true because the checksum is computed before lockedAt is attached and excludes it from the canonical payload. The persistence validator therefore also accepts the altered timestamp, allowing the claimed pre-registration time to be moved earlier or later without invalidating the lock; compute the checksum over the finalized metadata including lockedAt.

Useful? React with 👍 / 👎.

}

/**
* 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;
}
11 changes: 11 additions & 0 deletions packages/abtest/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions packages/abtest/src/operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines +160 to +162

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include hypothesis in the operation output schema

The AbTest model now includes hypothesis, but this schema extension only adds assignmentProvenance. When a persisted test contains hypothesis metadata, every CLI/MCP invoker passes the serialized test through parseOperationOutput; Zod strips the undeclared hypothesis property, so callers cannot retrieve the pre-registration even though it remains in storage. Add the hypothesis shape to this shared output schema.

AGENTS.md reference: AGENTS.md:L155-L157

Useful? React with 👍 / 👎.

Comment on lines +160 to +162

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Populate assignment provenance for persisted tests

The new assignmentProvenance field is parsed and serialized but never assigned anywhere in production: deterministic holdout provisioning sets assignmentManifest without setting manifest_v1, while full-split and hydrated legacy tests never receive legacy_unavailable. Consequently, every normal CLI/MCP response omits the field, so consumers cannot use it for its stated purpose of distinguishing deterministic manifests from legacy assignments; derive it when provisioning and when hydrating older records.

Useful? React with 👍 / 👎.

});

const testResultsSchema = z.object({
Expand Down
5 changes: 4 additions & 1 deletion packages/abtest/src/persistence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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")
);
}

Expand Down
4 changes: 4 additions & 0 deletions packages/abtest/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Validate persisted hypothesis records

Adding hypothesis to the persisted AbTest shape without extending isStoredAbTest means validateStoredAbTestStore and loadStoredAbTests accept arbitrary malformed hypothesis objects and post-lock checksum mismatches, after which parseAbTestStore casts them to HypothesisMetadata. Validate the nested shape and locked-state invariants before hydrating file-backed records so downstream launch/report code does not receive untrusted metadata.

AGENTS.md reference: AGENTS.md:L180-L183

Useful? React with 👍 / 👎.

Comment on lines +90 to +91

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Wire hypothesis metadata into test creation

When a test is created through the shared CLI/MCP operation, createAbTestInputSchema, CreateAbTestInput, and AbTestConfig provide no hypothesis field, and AbTestService.createTest provisions the assignment manifest without calling lockHypothesis. Consequently, users of either supported surface cannot pre-register this newly exposed metadata, despite it being returned in operation output; add the hypothesis to the shared creation contract and lock it before segmentation creates the manifest.

AGENTS.md reference: AGENTS.md:L146-L149

Useful? React with 👍 / 👎.

Comment on lines +90 to +91

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Use the pre-registered metric when selecting the winner

Adding hypothesis.primaryMetric does not affect analysis: analyzeStatisticalSignificance and winner selection still call pickMetricRate(results), which chooses conversion rate when any conversion exists and otherwise click rate, always preferring the largest value. A test pre-registered for revenue_per_recipient, or with direction: "minimize", can therefore analyze a different metric or select the opposite variant and subsequently auto-deploy it; the experiment report also labels the inferred click/conversion metric rather than the registered one.

Useful? React with 👍 / 👎.

Comment on lines +90 to +91

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Drive decisions from the pre-registered metric and direction

When a hypothesis selects revenue_per_recipient, or selects conversion_rate before any conversions occur, the existing AbTestService.pickMetricRate and buildExperimentReport paths still choose between click and conversion rates based only on observed conversions and always maximize the selected rate. Consequently, winner selection and reports can contradict both hypothesis.primaryMetric.type and a minimize direction, undermining the pre-registration guarantee; either use these fields throughout analysis or reject unsupported combinations.

Useful? React with 👍 / 👎.

/** 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
Expand Down
Loading