mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-11 14:50:02 +00:00
feat(studio): emit the canary reason on Studio events too
Review caught that the same anti-pattern was still live in the Studio binding: canaryEventProperties destructured only `enabled` and dropped the reason. Its own doc comment promised 'identical shape to the CLI, so a rollout spanning both reads as one flag' — which the CLI-only fix had just made false. This matters beyond symmetry. A CLI-launched Studio adopts the CLI's decisions and shares its bucket seed, so a cohort flip can surface on either surface. Emitting attribution on only one leaves Studio-observed flips unattributable and makes the two flip counts irreconcilable — and Studio is the surface most likely to expose a shared-seed-with-diverging-id pattern, which is the open question the reason exists to answer. Also adds the no_unit_id emission test the CLI side advertised but never asserted, and a Studio pair pinning that a URL override and a cohort roll produce the same assignment with different reasons. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
076657a639
commit
b3990ac789
@@ -267,6 +267,14 @@ describe("reason reaches the event properties", () => {
|
|||||||
expect(canaryEventProperties()["canary_reason_test_gamma"]).toBe("excluded");
|
expect(canaryEventProperties()["canary_reason_test_gamma"]).toBe("excluded");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// The PR body advertises no_unit_id as one of two values that pay for
|
||||||
|
// themselves; the resolver exercises it but the emission layer did not.
|
||||||
|
it("reports no_unit_id at the emission layer when the install has no id", () => {
|
||||||
|
configState.anonymousId = "";
|
||||||
|
configState.bucketSeed = undefined;
|
||||||
|
expect(canaryEventProperties()["canary_reason_test_gamma"]).toBe("no_unit_id");
|
||||||
|
});
|
||||||
|
|
||||||
it("reports telemetry_opt_out when the preference is off", () => {
|
it("reports telemetry_opt_out when the preference is off", () => {
|
||||||
configState.telemetryEnabled = false;
|
configState.telemetryEnabled = false;
|
||||||
// Emitted for completeness; by construction such an install sends nothing,
|
// Emitted for completeness; by construction such an install sends nothing,
|
||||||
|
|||||||
@@ -194,6 +194,8 @@ describe("telemetry", () => {
|
|||||||
expect(canaryEventProperties()).toEqual({
|
expect(canaryEventProperties()).toEqual({
|
||||||
"$feature/canary-on-everywhere": "true",
|
"$feature/canary-on-everywhere": "true",
|
||||||
"$feature/canary-off-everywhere": "false",
|
"$feature/canary-off-everywhere": "false",
|
||||||
|
canary_reason_on_everywhere: "in_cohort",
|
||||||
|
canary_reason_off_everywhere: "out_of_cohort",
|
||||||
});
|
});
|
||||||
|
|
||||||
__resetStudioCanaryCacheForTests();
|
__resetStudioCanaryCacheForTests();
|
||||||
@@ -235,6 +237,8 @@ describe("telemetry opt-out is canary opt-out", () => {
|
|||||||
expect(canaryEventProperties()).toEqual({
|
expect(canaryEventProperties()).toEqual({
|
||||||
"$feature/canary-on-everywhere": "false",
|
"$feature/canary-on-everywhere": "false",
|
||||||
"$feature/canary-off-everywhere": "false",
|
"$feature/canary-off-everywhere": "false",
|
||||||
|
canary_reason_on_everywhere: "telemetry_opt_out",
|
||||||
|
canary_reason_off_everywhere: "telemetry_opt_out",
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
@@ -323,3 +327,24 @@ describe("CLI-launched Studio adopts the CLI's decisions", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// A CLI-launched Studio shares the CLI's bucket seed, so a cohort flip can
|
||||||
|
// surface on either surface. Attribution on only one of them makes the two
|
||||||
|
// flip counts irreconcilable — which is why this is not a CLI-only property.
|
||||||
|
describe("Studio attribution matches the CLI", () => {
|
||||||
|
it("distinguishes a local URL override from a cohort roll at the same value", () => {
|
||||||
|
setSearch("?hf_canary_off_everywhere=on");
|
||||||
|
const props = canaryEventProperties();
|
||||||
|
expect(props["$feature/canary-off-everywhere"]).toBe("true");
|
||||||
|
expect(props["canary_reason_off_everywhere"]).toBe("forced_on");
|
||||||
|
// Same assignment `on-everywhere` reaches by an ordinary roll.
|
||||||
|
expect(props["$feature/canary-on-everywhere"]).toBe("true");
|
||||||
|
expect(props["canary_reason_on_everywhere"]).toBe("in_cohort");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("never puts a reason inside the $feature namespace", () => {
|
||||||
|
for (const [key, value] of Object.entries(canaryEventProperties())) {
|
||||||
|
if (key.startsWith("$feature/")) expect(value).toMatch(/^(true|false)$/);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -257,12 +257,22 @@ export function isCanaryEnabled(name: string): boolean {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Canary assignments as PostHog flag properties (`$feature/canary-<name>`),
|
* Canary assignments as PostHog flag properties (`$feature/canary-<name>`)
|
||||||
* attached to every Studio event so any metric can be split by cohort —
|
* plus their `canary_reason_<name>`, attached to every Studio event so any
|
||||||
* identical shape to the CLI, so a rollout spanning both reads as one flag.
|
* metric can be split by cohort — identical shape to the CLI, so a rollout
|
||||||
|
* spanning both reads as one flag.
|
||||||
|
*
|
||||||
|
* The reason has to be here too, not just CLI-side. A CLI-launched Studio
|
||||||
|
* adopts the CLI's decisions and shares its bucket seed, so a cohort flip can
|
||||||
|
* surface on either surface; emitting attribution on only one of them makes
|
||||||
|
* the two flip counts irreconcilable and leaves Studio-observed flips
|
||||||
|
* unattributable — the exact ambiguity this property exists to remove.
|
||||||
*/
|
*/
|
||||||
export function canaryEventProperties(): Record<string, string> {
|
export function canaryEventProperties(): Record<string, string> {
|
||||||
return canaryFeatureProperties(
|
return canaryFeatureProperties(
|
||||||
CANARIES.map((c) => ({ name: c.name, enabled: resolveCanary(c.name).enabled })),
|
CANARIES.map((c) => {
|
||||||
|
const { enabled, reason } = resolveCanary(c.name);
|
||||||
|
return { name: c.name, enabled, reason };
|
||||||
|
}),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user