From dccaa243825272207bf41950dd2c6fc5fb958098 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 26 Aug 2026 12:23:48 -0700 Subject: [PATCH 1/2] fix(telemetry)!: spell PostHog properties in snake_case PostHog's own properties are `$current_url` and `$lib_version`. Ours were a mix, which makes a property list unsearchable: you have to know which spelling a property happened to be born with before you can filter on it. Five renames, on the wire only: `cliVersion` -> `cli_version`, `scaffoldVersion` -> `scaffold_version`, `durationMs` -> `duration_ms`, `loggedIn` -> `logged_in`, `ruleCount` -> `rule_count`. `cli`, `anonymous`, `command`, `success` are single words; `gh_owner` was already correct. `groupIdentify`'s `groupType`/`groupKey` are the SDK's parameter names, not properties we choose, and are untouched. The TypeScript feeding those properties stays camelCase. `RunContext` is a domain type, not a wire format, so `duration_ms: context.durationMs` is the translation happening at the one boundary where it belongs rather than camelCase leaking outward or snake_case leaking inward. `taskless info --json` keeps its `loggedIn` field. Two wire formats with two audiences, each following its own ecosystem's convention, so the same value appearing as `loggedIn` in JSON and `logged_in` in telemetry is correct rather than a discrepancy to reconcile. The analytics spec now says so normatively, so the next property added does not have to rediscover it. Spec and tests move with the code. The `analytics` spec named `cliVersion` and `scaffoldVersion` in the standard-properties requirement and four scenarios, and `durationMs`/`loggedIn` in the cli_run requirement; leaving any of them would have left the spec asserting a name nothing emits. Its cli_run requirement already said `cli_version`, which is the inconsistency this fixes, now true rather than aspirational. Hard cut, no dual-emit. Historical events keep the old spellings permanently, so a query spanning the release needs both, and saved insights filtering on these five need updating by hand. Fixes #189 Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/posthog-snake-case-properties.md | 29 ++++++++++++ openspec/specs/analytics/spec.md | 51 +++++++++++++++++---- packages/cli/src/commands/rules.ts | 4 +- packages/cli/src/telemetry-run.ts | 8 ++-- packages/cli/src/telemetry.ts | 8 ++-- packages/cli/test/cli-run.test.ts | 13 +++--- packages/cli/test/telemetry.test.ts | 16 +++---- 7 files changed, 96 insertions(+), 33 deletions(-) create mode 100644 .changeset/posthog-snake-case-properties.md diff --git a/.changeset/posthog-snake-case-properties.md b/.changeset/posthog-snake-case-properties.md new file mode 100644 index 00000000..b57eb6f4 --- /dev/null +++ b/.changeset/posthog-snake-case-properties.md @@ -0,0 +1,29 @@ +--- +"@taskless/cli": patch +--- + +Telemetry property names are now snake_case, matching PostHog's own convention. + +Five properties are renamed on the wire: `cliVersion` becomes `cli_version`, +`scaffoldVersion` becomes `scaffold_version`, `durationMs` becomes +`duration_ms`, `loggedIn` becomes `logged_in`, and `ruleCount` becomes +`rule_count`. `cli`, `anonymous`, `command`, `success`, 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. Five is a cheap rename; fifteen would not have +been. + +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 five 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..a4c598d5 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,7 +172,7 @@ 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): @@ -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/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, }) ); From 0f704b7494b17659a66d34ead8b3901925fdd61b Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 26 Aug 2026 12:34:16 -0700 Subject: [PATCH 2/2] fix(telemetry)!: spell the check counts in snake_case too `cli_check_completed` carries `errorCount` and `warningCount`, built into an object forty lines before the `capture()` that sends them, which is why the audit behind the rename table missed both. The analytics spec named them as well. Folded into this change rather than left for a follow-up. The cost of the rename is not the edit, it is the historical discontinuity: events emitted before the release keep the old spelling forever, and saved insights need updating by hand. That cost is paid once per release. Renaming five now and two later pays it twice for the same total work, and this branch already carries the changeset and the dashboard-migration note that make it payable once. The locals stay camelCase. They are ordinary TypeScript, not a payload, and the translation happens at the wire boundary, matching how `duration_ms: context.durationMs` reads in `telemetry-run.ts`. --- .changeset/posthog-snake-case-properties.md | 18 +++++++++++------- openspec/specs/analytics/spec.md | 4 ++-- packages/cli/src/commands/check.ts | 11 +++++++++-- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/.changeset/posthog-snake-case-properties.md b/.changeset/posthog-snake-case-properties.md index b57eb6f4..f03f2d25 100644 --- a/.changeset/posthog-snake-case-properties.md +++ b/.changeset/posthog-snake-case-properties.md @@ -4,22 +4,26 @@ Telemetry property names are now snake_case, matching PostHog's own convention. -Five properties are renamed on the wire: `cliVersion` becomes `cli_version`, +Seven properties are renamed on the wire: `cliVersion` becomes `cli_version`, `scaffoldVersion` becomes `scaffold_version`, `durationMs` becomes -`duration_ms`, `loggedIn` becomes `logged_in`, and `ruleCount` becomes -`rule_count`. `cli`, `anonymous`, `command`, `success`, and `gh_owner` are -single words or already correct and are unchanged. +`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. Five is a cheap rename; fifteen would not have -been. +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 five need updating by hand at the same time; they +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. diff --git a/openspec/specs/analytics/spec.md b/openspec/specs/analytics/spec.md index a4c598d5..05c9b332 100644 --- a/openspec/specs/analytics/spec.md +++ b/openspec/specs/analytics/spec.md @@ -179,8 +179,8 @@ CLI events SHALL use the `cli_` prefix, with the taxonomy organized as a - `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` 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