From b3990ac789a40242b38eb740475811a03e701b2b Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Thu, 6 Aug 2026 14:39:56 -0700 Subject: [PATCH] feat(studio): emit the canary reason on Studio events too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- packages/cli/src/telemetry/canary.test.ts | 8 +++++++ packages/studio/src/telemetry/canary.test.ts | 25 ++++++++++++++++++++ packages/studio/src/telemetry/canary.ts | 18 ++++++++++---- 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/telemetry/canary.test.ts b/packages/cli/src/telemetry/canary.test.ts index 384a9d33d..235b3ff35 100644 --- a/packages/cli/src/telemetry/canary.test.ts +++ b/packages/cli/src/telemetry/canary.test.ts @@ -267,6 +267,14 @@ describe("reason reaches the event properties", () => { 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", () => { configState.telemetryEnabled = false; // Emitted for completeness; by construction such an install sends nothing, diff --git a/packages/studio/src/telemetry/canary.test.ts b/packages/studio/src/telemetry/canary.test.ts index 802a4aaf2..d3681ff6c 100644 --- a/packages/studio/src/telemetry/canary.test.ts +++ b/packages/studio/src/telemetry/canary.test.ts @@ -194,6 +194,8 @@ describe("telemetry", () => { expect(canaryEventProperties()).toEqual({ "$feature/canary-on-everywhere": "true", "$feature/canary-off-everywhere": "false", + canary_reason_on_everywhere: "in_cohort", + canary_reason_off_everywhere: "out_of_cohort", }); __resetStudioCanaryCacheForTests(); @@ -235,6 +237,8 @@ describe("telemetry opt-out is canary opt-out", () => { expect(canaryEventProperties()).toEqual({ "$feature/canary-on-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)$/); + } + }); +}); diff --git a/packages/studio/src/telemetry/canary.ts b/packages/studio/src/telemetry/canary.ts index d6240b2b7..c414d542d 100644 --- a/packages/studio/src/telemetry/canary.ts +++ b/packages/studio/src/telemetry/canary.ts @@ -257,12 +257,22 @@ export function isCanaryEnabled(name: string): boolean { } /** - * Canary assignments as PostHog flag properties (`$feature/canary-`), - * attached to every Studio event so any metric can be split by cohort — - * identical shape to the CLI, so a rollout spanning both reads as one flag. + * Canary assignments as PostHog flag properties (`$feature/canary-`) + * plus their `canary_reason_`, attached to every Studio event so any + * 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 { 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 }; + }), ); }