diff --git a/.changeset/posthog-snake-case-properties.md b/.changeset/posthog-snake-case-properties.md new file mode 100644 index 00000000..f03f2d25 --- /dev/null +++ b/.changeset/posthog-snake-case-properties.md @@ -0,0 +1,33 @@ +--- +"@taskless/cli": patch +--- + +Telemetry property names are now snake_case, matching PostHog's own convention. + +Seven properties are renamed on the wire: `cliVersion` becomes `cli_version`, +`scaffoldVersion` becomes `scaffold_version`, `durationMs` becomes +`duration_ms`, `loggedIn` becomes `logged_in`, `ruleCount` becomes +`rule_count`, and on `cli_check_completed`, `errorCount` and `warningCount` +become `error_count` and `warning_count`. `cli`, `anonymous`, `command`, +`success`, `findings`, and `gh_owner` are single words or already correct and +are unchanged. + +PostHog's own properties are spelled `$current_url` and `$lib_version`, so a +project that mixes the two conventions gives you a property list nobody can +search: you have to know which spelling a given property happened to be born +with before you can filter on it. Seven is a cheap rename; fifteen would not have +been, and the discontinuity below is paid once per release rather than once +per property, so splitting the set across two releases would have cost twice +for the same work. + +This is a hard cut, with no dual-emit window. **Historical events keep the old +spellings permanently**, so any PostHog query spanning this release needs both +names to see the whole series. Saved insights, dashboards, and cohorts that +filter or break down on these seven need updating by hand at the same time; they +will not error, they will silently return only the events emitted before the +release. + +The `loggedIn` field in `taskless info --json` is a different wire format with a +different audience and stays `camelCase`. JSON payloads follow the JSON +convention and telemetry properties follow PostHog's, so the same value +legitimately appears under both spellings. diff --git a/openspec/specs/analytics/spec.md b/openspec/specs/analytics/spec.md index 37451279..05c9b332 100644 --- a/openspec/specs/analytics/spec.md +++ b/openspec/specs/analytics/spec.md @@ -105,31 +105,62 @@ The PostHog client SHALL be created with `flushAt: 1` and `flushInterval: 0` bec - **WHEN** `shutdown()` is called - **THEN** all buffered events SHALL be flushed before the promise resolves +### Requirement: PostHog property names are snake_case + +Every property name attached to a PostHog `identify()` or `capture()` call SHALL +be `snake_case`. This is the naming convention of the PostHog ecosystem, whose +own properties (`$current_url`, `$lib_version`) are spelled that way, and mixing +conventions makes a property list unsearchable by anyone building an insight. + +JSON payloads the CLI emits to stdout (for example `taskless info --json`) are a +separate wire format with a separate audience, and SHALL remain `camelCase` per +the CLI spec. The same value MAY therefore appear as `loggedIn` in JSON and +`logged_in` in telemetry; that is each format following its own ecosystem's +convention, not a discrepancy to reconcile. + +A single-word property name (`cli`, `anonymous`, `command`, `success`) satisfies +this requirement unchanged. Parameters of the PostHog SDK itself, such as +`groupIdentify`'s `groupType` and `groupKey`, are not properties the CLI names +and are out of scope. + +#### Scenario: A multi-word telemetry property is snake_case + +- **WHEN** a `capture()` call carries a property whose name has more than one word +- **THEN** the property name SHALL be spelled `snake_case` (for example + `cli_version`, `scaffold_version`, `duration_ms`, `logged_in`, `rule_count`) + +#### Scenario: The JSON field and the telemetry property may differ in spelling + +- **WHEN** the same value is reported both in `taskless info --json` and as a + telemetry property +- **THEN** the JSON field SHALL be `camelCase` (`loggedIn`) and the telemetry + property SHALL be `snake_case` (`logged_in`) + ### Requirement: All capture calls include standard properties -Every `capture()` call SHALL include the `cli` property (anonymous UUID), the `cliVersion` property (the version the build reports as its own, baked in at build time), and the `scaffoldVersion` property (the `version` field from `.taskless/taskless.json`, or `0` if the manifest is absent or unreadable). When authenticated, the `groups` parameter SHALL include `{ organization: String(orgId) }`. The `cliVersion` and `scaffoldVersion` values SHALL be resolved once at telemetry initialization and attached to every subsequent `capture()` call without re-reading the source files. +Every `capture()` call SHALL include the `cli` property (anonymous UUID), the `cli_version` property (the version the build reports as its own, baked in at build time), and the `scaffold_version` property (the `version` field from `.taskless/taskless.json`, or `0` if the manifest is absent or unreadable). When authenticated, the `groups` parameter SHALL include `{ organization: String(orgId) }`. The `cli_version` and `scaffold_version` values SHALL be resolved once at telemetry initialization and attached to every subsequent `capture()` call without re-reading the source files. #### Scenario: Anonymous capture includes standard properties - **WHEN** `capture("cli_run")` is called without authentication -- **THEN** the event SHALL include `{ cli: anonymousUuid, cliVersion: , scaffoldVersion: }` +- **THEN** the event SHALL include `{ cli: anonymousUuid, cli_version: , scaffold_version: }` - **AND** the event SHALL NOT include a `groups` parameter #### Scenario: Authenticated capture includes standard properties and group - **WHEN** `capture("cli_rule_created")` is called with authentication -- **THEN** the event SHALL include `{ cli: anonymousUuid, cliVersion: , scaffoldVersion: }` +- **THEN** the event SHALL include `{ cli: anonymousUuid, cli_version: , scaffold_version: }` - **AND** the `groups` parameter SHALL include `{ organization: String(orgId) }` #### Scenario: Scaffold version falls back to 0 when manifest missing - **WHEN** `getTelemetry(cwd)` is initialized in a directory with no `.taskless/taskless.json` -- **THEN** every `capture()` call from the returned client SHALL include `scaffoldVersion: 0` +- **THEN** every `capture()` call from the returned client SHALL include `scaffold_version: 0` #### Scenario: CLI version is the version the build reports as its own - **WHEN** `getTelemetry()` is initialized -- **THEN** `cliVersion` SHALL be the version the build reports as its own, bundled at build time +- **THEN** `cli_version` SHALL be the version the build reports as its own, bundled at build time - **AND** for every build target but `nightly` that SHALL be the version in `packages/cli/package.json` - **AND** for a `nightly` it SHALL be the version the nightly is published under, so events are attributed to the build that emitted them rather than to the release it anticipates - **AND** SHALL be attached to every event emitted through the returned client @@ -141,15 +172,15 @@ CLI events SHALL use the `cli_` prefix, with the taxonomy organized as a - `cli_run` — exactly one per invocation (see the dedicated requirement). This replaces every previous `cli_` start event and `cli__completed` - event; the `success`/`durationMs`/`command` signal lives here. + event; the `success`/`duration_ms`/`command` signal lives here. - Concrete state-transition events, each fired at the point the state actually changes, carrying counts/ids/booleans only (never rule content, prompts, or matched source): - `cli_rule_created`, `cli_rule_improved`, `cli_rule_deleted` - `cli_authenticated`, `cli_logged_out` - `cli_installed`, `cli_onboarded` - - `cli_check_completed` — error/warning counts only (e.g. `errorCount`, - `warningCount`, `findings`) + - `cli_check_completed` — error/warning counts only (e.g. `error_count`, + `warning_count`, `findings`) - `cli_error` — a single failure event with `command` and `code` (a stable `CLIErrorCode`) - `cli_agent` — fired when the `agent` command serves a request, with a `topic` @@ -257,8 +288,8 @@ Each command handler SHALL call `getTelemetry(cwd)` to lazily initialize the sin The CLI SHALL emit exactly one `cli_run` event per invocation, from the top-level runner rather than from individual commands. The event SHALL carry the properties `command` (the resolved subcommand name, e.g. `"rule create"` or `"help"`), -`cli_version`, `success` (boolean), `durationMs` (number), `anonymous` (boolean), -and `loggedIn` (boolean). The event SHALL be emitted on both success and failure +`cli_version`, `success` (boolean), `duration_ms` (number), `anonymous` (boolean), +and `logged_in` (boolean). The event SHALL be emitted on both success and failure (from a `finally`-equivalent path), and no command SHALL emit its own "started" or "ran" event. @@ -266,8 +297,8 @@ and `loggedIn` (boolean). The event SHALL be emitted on both success and failure - **WHEN** a user runs `taskless info` - **THEN** PostHog SHALL receive exactly one `cli_run` event with - `command: "info"`, `success: true`, a numeric `durationMs`, and the - `cli_version`, `anonymous`, and `loggedIn` properties + `command: "info"`, `success: true`, a numeric `duration_ms`, and the + `cli_version`, `anonymous`, and `logged_in` properties - **AND** SHALL NOT receive a separate `cli_info` or `cli_info_completed` event #### Scenario: A failing command still emits cli_run diff --git a/packages/cli/src/commands/check.ts b/packages/cli/src/commands/check.ts index d6c5b87c..60432685 100644 --- a/packages/cli/src/commands/check.ts +++ b/packages/cli/src/commands/check.ts @@ -264,7 +264,7 @@ export const checkCommand = defineCommand({ // Set when a scan actually runs; drives cli_check_completed with counts // only (never matched code). let scanCounts: - | { errorCount: number; warningCount: number; findings: number } + | { error_count: number; warning_count: number; findings: number } | undefined; try { const positionalPaths = extractPositionalPaths(rawArgs); @@ -386,7 +386,14 @@ export const checkCommand = defineCommand({ if (result.severity === "error") errorCount++; else if (result.severity === "warning") warningCount++; } - scanCounts = { errorCount, warningCount, findings: results.length }; + // snake_case at the wire boundary: these become PostHog properties + // verbatim. The locals stay camelCase because they are ordinary + // TypeScript, not a payload. + scanCounts = { + error_count: errorCount, + warning_count: warningCount, + findings: results.length, + }; // Computed by `runEngines`, not here: the exit code is a fact about a // completed dispatch, and an engine failure has to fail the check even diff --git a/packages/cli/src/commands/rules.ts b/packages/cli/src/commands/rules.ts index 14cf647a..f5fa146b 100644 --- a/packages/cli/src/commands/rules.ts +++ b/packages/cli/src/commands/rules.ts @@ -289,7 +289,7 @@ const createCommand = defineCommand({ } finally { // Concrete state event: a rule was actually generated and written. if (createdRuleCount !== undefined) { - telemetry.capture("cli_rule_created", { ruleCount: createdRuleCount }); + telemetry.capture("cli_rule_created", { rule_count: createdRuleCount }); } } }, @@ -528,7 +528,7 @@ const improveCommand = defineCommand({ // Concrete state event: a rule was actually iterated and rewritten. if (improvedRuleCount !== undefined) { telemetry.capture("cli_rule_improved", { - ruleCount: improvedRuleCount, + rule_count: improvedRuleCount, }); } } diff --git a/packages/cli/src/telemetry-run.ts b/packages/cli/src/telemetry-run.ts index 58ebf11a..a83946f9 100644 --- a/packages/cli/src/telemetry-run.ts +++ b/packages/cli/src/telemetry-run.ts @@ -52,7 +52,7 @@ export interface RunContext { /** * Emit the per-invocation telemetry: a single `cli_run` denominator event * (always), preceded by `cli_error` only when the command threw. The CLI - * version is NOT added here — it rides on the standard `cliVersion` property + * version is NOT added here — it rides on the standard `cli_version` property * the telemetry client attaches to every event. */ export function emitRunEvents( @@ -71,11 +71,13 @@ export function emitRunEvents( telemetry.capture("cli_error", { command: context.command, code }); } + // PostHog property names are snake_case; the RunContext fields feeding them + // stay camelCase because they are TypeScript, not wire format. telemetry.capture("cli_run", { command: context.command, success: context.success, - durationMs: context.durationMs, + duration_ms: context.durationMs, anonymous: context.anonymous, - loggedIn: context.loggedIn, + logged_in: context.loggedIn, }); } diff --git a/packages/cli/src/telemetry.ts b/packages/cli/src/telemetry.ts index 43855730..ffdfcaaf 100644 --- a/packages/cli/src/telemetry.ts +++ b/packages/cli/src/telemetry.ts @@ -193,8 +193,8 @@ export async function getTelemetry(cwd?: string): Promise { distinctId, properties: { cli: anonymousId, - cliVersion: CLI_VERSION, - scaffoldVersion, + cli_version: CLI_VERSION, + scaffold_version: scaffoldVersion, gh_owner: ghOwner, }, }); @@ -217,8 +217,8 @@ export async function getTelemetry(cwd?: string): Promise { properties: { ...properties, cli: anonymousId, - cliVersion: CLI_VERSION, - scaffoldVersion, + cli_version: CLI_VERSION, + scaffold_version: scaffoldVersion, gh_owner: ghOwner, }, ...(!anonymous && orgSubject !== undefined diff --git a/packages/cli/test/cli-run.test.ts b/packages/cli/test/cli-run.test.ts index c6a1ebe2..f9e14a55 100644 --- a/packages/cli/test/cli-run.test.ts +++ b/packages/cli/test/cli-run.test.ts @@ -25,7 +25,7 @@ function fakeTelemetry() { const anon = { anonymous: true, loggedIn: false }; describe("emitRunEvents", () => { - it("emits exactly one cli_run on success, with no cli_error and no cli_version", () => { + it("emits exactly one cli_run on success, with no cli_error and no version property", () => { const telemetry = fakeTelemetry(); emitRunEvents(telemetry, { command: "info", @@ -40,12 +40,13 @@ describe("emitRunEvents", () => { expect.objectContaining({ command: "info", success: true, - durationMs: 5, + duration_ms: 5, anonymous: true, - loggedIn: false, + logged_in: false, }) ); - // Version rides on the standard cliVersion property, not a cli_version field. + // Version rides on the standard cli_version property the telemetry client + // attaches, so emitRunEvents must not add one of its own. const properties = telemetry.capture.mock.calls[0]![1] as Record< string, unknown @@ -112,7 +113,7 @@ describe("emitRunEvents", () => { expect(events).not.toContain("cli_error"); }); - it("reflects an authenticated identity as loggedIn", () => { + it("reflects an authenticated identity as logged_in", () => { const telemetry = fakeTelemetry(); emitRunEvents(telemetry, { command: "info", @@ -124,7 +125,7 @@ describe("emitRunEvents", () => { expect(telemetry.capture).toHaveBeenCalledWith( "cli_run", - expect.objectContaining({ anonymous: false, loggedIn: true }) + expect.objectContaining({ anonymous: false, logged_in: true }) ); }); }); diff --git a/packages/cli/test/telemetry.test.ts b/packages/cli/test/telemetry.test.ts index 698a3890..32cfaaf5 100644 --- a/packages/cli/test/telemetry.test.ts +++ b/packages/cli/test/telemetry.test.ts @@ -296,21 +296,21 @@ describe("capture", () => { expect(captureArgument).not.toHaveProperty("groups"); }); - it("includes cliVersion and scaffoldVersion on every anonymous capture", async () => { + it("includes cli_version and scaffold_version on every anonymous capture", async () => { const telemetry = await getTelemetry(); telemetry.capture("cli_run"); expect(mockCapture).toHaveBeenCalledWith( expect.objectContaining({ properties: expect.objectContaining({ - cliVersion: expect.any(String) as string, - scaffoldVersion: expect.any(Number) as number, + cli_version: expect.any(String) as string, + scaffold_version: expect.any(Number) as number, }) as Record, }) ); }); - it("includes cliVersion and scaffoldVersion on authenticated capture", async () => { + it("includes cli_version and scaffold_version on authenticated capture", async () => { const cwd = await mkdtemp(join(tmpdir(), "taskless-auth-props-")); try { // Seed a manifest with a known scaffold version @@ -331,8 +331,8 @@ describe("capture", () => { expect.objectContaining({ groups: { organization: "123" }, properties: expect.objectContaining({ - cliVersion: expect.any(String) as string, - scaffoldVersion: 2, + cli_version: expect.any(String) as string, + scaffold_version: 2, }) as Record, }) ); @@ -341,7 +341,7 @@ describe("capture", () => { } }); - it("falls back to scaffoldVersion: 0 when manifest is missing", async () => { + it("falls back to scaffold_version: 0 when manifest is missing", async () => { const cwd = await mkdtemp(join(tmpdir(), "taskless-no-manifest-")); try { const telemetry = await getTelemetry(cwd); @@ -350,7 +350,7 @@ describe("capture", () => { expect(mockCapture).toHaveBeenCalledWith( expect.objectContaining({ properties: expect.objectContaining({ - scaffoldVersion: 0, + scaffold_version: 0, }) as Record, }) );