From f822200fb8f437b73985f43837c195f5e7312289 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Thu, 20 Aug 2026 18:26:14 -0400 Subject: [PATCH] feat(telemetry): measure which lint rules fire, cost, and fail to converge (#3367) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(telemetry): measure which lint rules fire, cost, and fail to converge Lint rule changes are currently argued from anecdote. This adds the three measurements needed to argue them from data. `lint_report`, once per `hyperframes lint` or `hyperframes check`: - `code_counts` / `codes` — which rules actually fire, and how often - `rule_group_ms` — milliseconds per rule-source module (core, gsap, media, ...) - `slowest_rule` / `slowest_rule_ms` — slowest single rule as `#` - `rule_count` — how many rules this build ran `lint_rule_streak`, once per finding that survives an edit to its file: - `edits` — how many edits the finding survived - `cleared` — whether it eventually went away The streak event is the one that matters. A lint pass costs about 5ms, so per-rule CPU is not what makes the authoring loop slow; a rule an agent cannot satisfy is, because every failed attempt costs a full edit-and-relint cycle. A single run cannot see that, so `lint_rule_streak` reconstructs it across runs: high `edits` with `cleared: false` is a rule nobody can fix, and the `cleared: true` distribution is the baseline to judge it against. An iteration is counted only when the file's content digest CHANGED and the finding is still there. Re-linting an untouched project is not an attempt, which is what stops `check` (which lints on every invocation) from inflating the numbers. Rule identity is the source module plus an index within it. Naming all 86 rules would make the timings prettier but it is a refactor this measurement does not need: the group locates the file, and the index locates the rule. Version, agent runtime, CI flag, and invocation id are already attached to every event by `trackEvent`, so lint pain can be split by CLI version and by which agent produced it without adding anything here. Privacy: only rule codes, counts, and timings are sent. Streak state lives in ~/.hyperframes/lint-streaks.json alongside config.json (so `rm -rf ~/.hyperframes` is still a full reset) and stores digests only — no file paths, no project names, no composition source. Nothing is written and nothing is emitted when telemetry is off. Entries expire after 14 days and are capped at 500 files. `EventProperties` gains string arrays and numeric maps. `codes` and `code_counts` are inherently a set and a histogram; flattening them into dynamic top-level keys would make them unqueryable. PostHog stores both natively. `trackLintRun` is the single call site shared by `lint` and `check`, and it swallows every error — telemetry must never turn a green lint red. * feat(telemetry): emit per-group rule counts so slowest_rule stays comparable Review catch on #3367: `slowest_rule` is the one positional key in either event. It is `#`, so adding or removing a rule renumbers every later slot in that group and the same string means different rules in two builds. #3366 does exactly that to 34 of 81 surviving slots, and `rule_count` alone says only THAT the ruleset moved, not which groups. `rule_group_counts` carries the per-group sizes alongside it, so a consumer comparing two builds can tell which groups' indices still mean the same thing without anyone having to remember which release dropped rules. `codes`, `code_counts` and `rule_group_ms` are keyed by name and were never affected. Also corrects the rule count in the RULE_GROUPS comment: 86, not ~60, as LINT_RULE_COUNT in the same file computes. --- packages/cli/src/commands/check.test.ts | 2 + packages/cli/src/commands/lint.ts | 8 + packages/cli/src/commands/validate.test.ts | 3 + packages/cli/src/telemetry/events.ts | 73 ++++++++ .../cli/src/telemetry/lintRun.e2e.test.ts | 134 +++++++++++++++ packages/cli/src/telemetry/lintRun.ts | 83 +++++++++ .../cli/src/telemetry/lintStreaks.test.ts | 161 ++++++++++++++++++ packages/cli/src/telemetry/lintStreaks.ts | Bin 0 -> 7432 bytes packages/cli/src/telemetry/transport.ts | 16 +- packages/cli/src/utils/checkPipeline.ts | 7 + packages/cli/src/utils/lintFormat.test.ts | 1 + packages/lint/src/hyperframeLinter.ts | 141 ++++++++++++--- packages/lint/src/index.ts | 8 +- packages/lint/src/project.ts | 24 ++- packages/lint/src/types.ts | 13 ++ 15 files changed, 640 insertions(+), 34 deletions(-) create mode 100644 packages/cli/src/telemetry/lintRun.e2e.test.ts create mode 100644 packages/cli/src/telemetry/lintRun.ts create mode 100644 packages/cli/src/telemetry/lintStreaks.test.ts create mode 100644 packages/cli/src/telemetry/lintStreaks.ts diff --git a/packages/cli/src/commands/check.test.ts b/packages/cli/src/commands/check.test.ts index b5a86f73f..6ea45395d 100644 --- a/packages/cli/src/commands/check.test.ts +++ b/packages/cli/src/commands/check.test.ts @@ -58,6 +58,7 @@ function cleanLint(): ProjectLintResult { results: [ { file: "index.html", + contentHash: "test", result: { ok: true, errorCount: 0, @@ -82,6 +83,7 @@ function lintWith( results: [ { file: "index.html", + contentHash: "test", result: { ok: severity !== "error", errorCount: severity === "error" ? 1 : 0, diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts index b592fd1e5..149ea9668 100644 --- a/packages/cli/src/commands/lint.ts +++ b/packages/cli/src/commands/lint.ts @@ -12,6 +12,8 @@ export const examples: Example[] = [ import { formatLintFindings } from "../utils/lintFormat.js"; import { lintProject } from "../utils/lintProject.js"; import { resolveProject } from "../utils/project.js"; +import { trackLintRun } from "../telemetry/lintRun.js"; +import { getRunId } from "../telemetry/runId.js"; import { withMeta } from "../utils/updateCheck.js"; export default defineCommand({ @@ -45,7 +47,13 @@ export default defineCommand({ // (publish/transcribe/upgrade/play/present) already use. try { const project = resolveProject(args.dir); + const startedAt = Date.now(); const lintResult = await lintProject(project.dir); + trackLintRun(project.dir, lintResult, { + command: "lint", + durationMs: Date.now() - startedAt, + ...(getRunId() !== undefined ? { runId: getRunId() } : {}), + }); if (args.json) { const allFindings = lintResult.results.flatMap((r) => r.result.findings); diff --git a/packages/cli/src/commands/validate.test.ts b/packages/cli/src/commands/validate.test.ts index f706181ef..2d78f6b39 100644 --- a/packages/cli/src/commands/validate.test.ts +++ b/packages/cli/src/commands/validate.test.ts @@ -232,6 +232,7 @@ describe("extractCompositionErrorsFromLint", () => { results: [ { file: "index.html", + contentHash: "test", result: { ok: findings.length === 0, errorCount: 0, @@ -282,6 +283,7 @@ describe("extractCompositionErrorsFromLint", () => { results: [ { file: "index.html", + contentHash: "test", result: { ok: false, errorCount: 1, @@ -298,6 +300,7 @@ describe("extractCompositionErrorsFromLint", () => { }, { file: "compositions/nested.html", + contentHash: "test", result: { ok: false, errorCount: 1, diff --git a/packages/cli/src/telemetry/events.ts b/packages/cli/src/telemetry/events.ts index 8e9b6420f..02b01701f 100644 --- a/packages/cli/src/telemetry/events.ts +++ b/packages/cli/src/telemetry/events.ts @@ -819,3 +819,76 @@ export function trackCheckReport(props: { ...runIdField(props.runId), }); } + +/** + * One lint pass over a project. `code_counts` is what makes "which rules + * actually fire" answerable; `rule_group_ms` and `slowest_rule` are what make + * "which rules are expensive" answerable. Only lint rule codes and timings are + * sent — never file paths, project names, or composition source. + */ +export function trackLintReport(props: { + /** The command that ran the lint: "lint" or "check". */ + command: string; + durationMs: number; + filesScanned: number; + errorCount: number; + warningCount: number; + infoCount: number; + /** Finding count keyed by lint rule code. */ + codeCounts: Record; + /** Milliseconds spent per rule-source module, summed across files. */ + ruleGroupMs: Record; + /** Slowest single rule as `#`, across every file in the run. */ + slowestRule: string; + slowestRuleMs: number; + /** How many rules this build ran, so a ruleset change is visible in the data. */ + ruleCount: number; + /** + * Rule count per group. `slowest_rule` is positional, so a group that changed + * size between two builds has indices that no longer mean the same thing. + */ + ruleGroupCounts: Record; + runId?: string; +}): void { + trackEvent("lint_report", { + command: props.command, + duration_ms: Math.round(props.durationMs), + files_scanned: props.filesScanned, + error_count: props.errorCount, + warning_count: props.warningCount, + info_count: props.infoCount, + codes: Object.keys(props.codeCounts).sort(), + code_counts: props.codeCounts, + rule_group_ms: props.ruleGroupMs, + slowest_rule: props.slowestRule, + slowest_rule_ms: Math.round(props.slowestRuleMs), + rule_count: props.ruleCount, + rule_group_counts: props.ruleGroupCounts, + ...runIdField(props.runId), + }); +} + +/** + * A finding that survived one or more edits to the file it was reported on. + * + * `cleared: false` with a high `edits` is the signal that matters most: a rule + * an agent kept trying and failing to satisfy. `cleared: true` gives the + * distribution to compare it against — how many edits a normal finding costs. + */ +export function trackLintRuleStreak(props: { + code: string; + severity: string; + edits: number; + cleared: boolean; + command: string; + runId?: string; +}): void { + trackEvent("lint_rule_streak", { + code: props.code, + severity: props.severity, + edits: props.edits, + cleared: props.cleared, + command: props.command, + ...runIdField(props.runId), + }); +} diff --git a/packages/cli/src/telemetry/lintRun.e2e.test.ts b/packages/cli/src/telemetry/lintRun.e2e.test.ts new file mode 100644 index 000000000..93d715fc4 --- /dev/null +++ b/packages/cli/src/telemetry/lintRun.e2e.test.ts @@ -0,0 +1,134 @@ +// End-to-end: a real project on disk -> lintProject -> the exact PostHog +// payloads. Unit tests cover the streak arithmetic; this proves the wiring +// and pins the event shape a dashboard will be built against. + +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +const HOME = mkdtempSync(join(tmpdir(), "hf-lintrun-")); +vi.mock("node:os", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, homedir: () => HOME }; +}); + +// Capture at the transport boundary so everything client.ts adds +// (cli_version, invocation_id, ...) is visible in the assertions. +const enqueued: Array<{ event: string; properties: Record }> = []; +vi.mock("./transport.js", () => ({ + enqueue: (event: string, properties: Record) => + enqueued.push({ event, properties }), + flush: () => Promise.resolve(), +})); +// shouldTrack() consults these two. A dev build disables telemetry by default, +// which would make this test assert on an empty queue and pass for the wrong +// reason. +vi.mock("./policy.js", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, telemetryRuntimeOverride: () => null }; +}); +vi.mock("./config.js", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, readConfig: () => ({ ...actual.readConfig(), telemetryEnabled: true }) }; +}); + +const { trackLintRun } = await import("./lintRun.js"); +const { lintProject } = await import("@hyperframes/lint"); + +const COMPOSITION = ` +
+ +
+ + +`; + +function makeProject(html: string): string { + const dir = mkdtempSync(join(tmpdir(), "hf-proj-")); + mkdirSync(join(dir, "compositions"), { recursive: true }); + writeFileSync(join(dir, "index.html"), html, "utf-8"); + return dir; +} + +beforeEach(() => { + enqueued.length = 0; + rmSync(join(HOME, ".hyperframes"), { recursive: true, force: true }); +}); + +describe("trackLintRun end to end", () => { + it("emits one lint_report carrying codes, timings, and the ruleset fingerprint", async () => { + const dir = makeProject(COMPOSITION); + const result = await lintProject(dir); + trackLintRun(dir, result, { command: "lint", durationMs: 12 }); + + const reports = enqueued.filter((e) => e.event === "lint_report"); + expect(reports).toHaveLength(1); + const props = reports[0]!.properties; + + expect(props["command"]).toBe("lint"); + expect(props["files_scanned"]).toBe(1); + expect(props["duration_ms"]).toBe(12); + // The real linter found real problems in this composition. + expect((props["codes"] as string[]).length).toBeGreaterThan(0); + expect(props["error_count"]).toBeGreaterThan(0); + // Timings are attributed per rule group and a slowest rule is identified. + expect(Object.keys(props["rule_group_ms"] as object)).toContain("gsap"); + expect(props["slowest_rule"]).toMatch(/^[a-z]+#\d+$/); + // Version and ruleset fingerprint ride along. + expect(props["cli_version"]).toBeTruthy(); + expect(props["rule_count"]).toBeGreaterThan(0); + // Per-group sizes make the positional `slowest_rule` index comparable + // across builds: a group that changed size renumbered its rules. + const groupCounts = props["rule_group_counts"] as Record; + expect(groupCounts["gsap"]).toBeGreaterThan(0); + expect(Object.values(groupCounts).reduce((a, b) => a + b, 0)).toBe(props["rule_count"]); + // code_counts sums to the number of findings. + const counts = Object.values(props["code_counts"] as Record); + expect(counts.reduce((a, b) => a + b, 0)).toBe( + result.results.flatMap((r) => r.result.findings).length, + ); + + rmSync(dir, { recursive: true, force: true }); + }); + + it("emits lint_rule_streak with cleared:true once an edit removes the finding", async () => { + const dir = makeProject(COMPOSITION); + + const first = await lintProject(dir); + trackLintRun(dir, first, { command: "lint", durationMs: 1 }); + const codes = first.results[0]!.result.findings.map((f) => f.code); + expect(codes).toContain("media_missing_data_start"); + + // Fix exactly that finding and re-lint. + writeFileSync( + join(dir, "index.html"), + COMPOSITION.replace('