Fix the published lab types and freeze the form-factor constants

.github/workflows/types.yml

@@ -0,0 +1,35 @@
+name: Types
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+    branches: [main]
+
+jobs:
+  types:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-node@v4
+        with:
+          node-version: 22
+          cache: npm
+
+      - run: npm ci
+
+      # The committed declarations must match the current JSDoc. Without this, check-types
+      # would happily compile against a stale types/ and report green while the published
+      # declarations drift away from the implementation.
+      - name: Declarations are up to date
+        run: |
+          npm run generate-types
+          git diff --exit-code -- types/ \
+            || { echo "::error::types/ is stale — run 'npm run generate-types' and commit the result"; exit 1; }
+
+      # npm test exercises the implementation, not the .d.ts. This compiles a sample consumer
+      # against the published declarations, resolving through package.json "exports" exactly
+      # as a consumer's build would.
+      - run: npm run check-types

CLAUDE.md

@@ -185,12 +185,19 @@ Run these in order at the end of every task, without exception:
 npm run lint          # must pass before running tests
 npm test              # all tests must pass
 npm run generate-types  # regenerate types after any function signature change
+npm run check-types     # type-check a consumer against the regenerated .d.ts
 ```
 
+`check-types` runs last because it reads what `generate-types` just emitted. It compiles
+`type-tests/consumer.ts`, which imports the package by name and so resolves through
+`package.json` "exports" exactly as a consumer's build would.
+
 ### Rules
 
 **JSDoc** — Any change to a function's parameters or return value requires updating its `@param` / `@returns` JSDoc. The generated `.d.ts` is the source of truth for consumers; stale types are bugs.
 
+`npm test` does not check the `.d.ts` files — it exercises the implementation. `npm run check-types` is what checks them, and a published type can be wrong while every test passes: `LabPlanOptions` once rejected every option the CLI itself passes. When a change adds an option, a return field, or a hook argument, extend `type-tests/consumer.ts` to use it, or the guard will not cover it.
+
 **New lib modules** — two steps, and only the first is automatic.
 
 1. `tsconfig.types.json` → `include` array. **Always.** `generate-types` only emits a `.d.ts` for what is listed, and a private module still needs one when another module's published types reference it.

README.md

@@ -549,6 +549,10 @@ Key exported types:
 | `CruxBatchResult` | `{ url, data: CruxReport \| null, error: string \| null }` |
 | `CruxHistoryBatchResult` | `{ url, data: CruxHistoryReport \| null, error: string \| null }` |
 | `LabPlanResult` | `{ url, profile, outputPath?, error? }` — one per run in a `runLabPlan` plan |
+| `LabAuditOptions` | Options for a single `runLabAudit` call (`port`, `profile`, `network`, `device`, `categories`, `skipAudits`, `blockedUrlPatterns`, `stripJsonProps`, `silent`) |
+| `LabWriteOptions` | `LabAuditOptions` plus `clean` — what `runLabToDisk` takes |
+| `LabPlanControls` | Plan-level controls only: `continueOnError`, `reuseBrowser`, `repeats` |
+| `LabPlanOptions` | `LabPlanControls` plus the per-run options, minus `runNumber` and `port`, which `runLabPlan` owns |
 | `RunSummary` | Variance record for one repeated (URL x profile) pair: median, spread, `benchmarkIndex` range, per-metric arrays, stability warnings |
 
 ## Development
@@ -570,9 +574,11 @@ node bin/web-perf.js lab https://example.com
 | `npm run lint` | Lint and auto-fix with ESLint |
 | `npm test` | Run all tests (vitest) |
 | `npm run generate-types` | Regenerate `types/lib/*.d.ts` from JSDoc annotations |
+| `npm run check-types` | Type-check a sample consumer against the generated declarations |
 
-Run them in that order at the end of every change — `lint` must pass before `test`, and
-`generate-types` last so the regenerated `.d.ts` reflects the final JSDoc.
+Run them in that order at the end of every change — `lint` must pass before `test`,
+`generate-types` after that so the regenerated `.d.ts` reflects the final JSDoc, and
+`check-types` last, since it compiles a sample consumer against what `generate-types` emitted.
 
 ### Regenerating types
 

lib/crux-client.js

@@ -5,8 +5,16 @@ const {
 } = require('./utils');
 
 const CRUX_MAX_REQUESTS_PER_SECOND = 2.5;
-const CRUX_FORM_FACTORS = /** @type {const} */ (['phone', 'desktop', 'tablet']);
-const DEFAULT_CRUX_FORM_FACTORS = ['phone', 'desktop'];
+// Frozen because both are exported from crux and crux-history as the same instance, and
+// DEFAULT_CRUX_FORM_FACTORS is also the default parameter value inside the client: a consumer
+// pushing to it would silently add a form factor to every later call in the process, against
+// a metered quota.
+//
+// Both carry an element type, not bare string[], and the formFactors options accept a
+// readonly array — otherwise the exported constants could not be passed to the very functions
+// they are the defaults for. Callers extending them spread first, as lib/prompts.js does.
+const CRUX_FORM_FACTORS = Object.freeze(/** @type {const} */ (['phone', 'desktop', 'tablet']));
+const DEFAULT_CRUX_FORM_FACTORS = Object.freeze(/** @type {readonly CruxFormFactor[]} */ (['phone', 'desktop']));
 
 /**
  * @typedef {'phone'|'desktop'|'tablet'} CruxFormFactor
@@ -23,15 +31,15 @@ const DEFAULT_CRUX_FORM_FACTORS = ['phone', 'desktop'];
  *   CRUX_HISTORY_MAX_REQUESTS_PER_SECOND inert, so editing it would have changed nothing.
  *
  * @typedef {{ scope?: 'origin'|'page', formFactor?: CruxFormFactor }} CruxAuditOptions
- * @typedef {{ scope?: 'origin'|'page', formFactors?: CruxFormFactor[], onNoData?: (formFactor: CruxFormFactor, message: string) => void }} CruxRunOptions
+ * @typedef {{ scope?: 'origin'|'page', formFactors?: readonly CruxFormFactor[], onNoData?: (formFactor: CruxFormFactor, message: string) => void }} CruxRunOptions
  */
 
 /**
  * @typedef {Object} CruxBatchOptions
  * @property {'origin'|'page'} [scope]
  * @property {number} [concurrency]
  * @property {number} [delayMs]
- * @property {CruxFormFactor[]} [formFactors]
+ * @property {readonly CruxFormFactor[]} [formFactors]
  * @property {(completed: number, total: number, url: string, error: string|null, statusCode: number|null) => void} [onProgress]
  */
 

lib/crux-client.test.js

@@ -180,3 +180,29 @@ describe('crux and crux-history are wired to different endpoints', () => {
         await expect(runCruxHistoryAudit('https://example.com', 'KEY')).rejects.toThrow('No CrUX history data found');
     });
 });
+
+describe('exported form-factor constants are immutable', () => {
+    const crux = require('./crux');
+    const cruxHistory = require('./crux-history');
+
+    it('freezes both constants', () => {
+        expect(Object.isFrozen(crux.CRUX_FORM_FACTORS)).toBe(true);
+        expect(Object.isFrozen(crux.DEFAULT_CRUX_FORM_FACTORS)).toBe(true);
+    });
+
+    // They are deliberately one instance shared by both subpaths, which is exactly why a
+    // consumer mutating one would have changed the default for the other.
+    it('shares one instance across crux and crux-history', () => {
+        expect(crux.DEFAULT_CRUX_FORM_FACTORS).toBe(cruxHistory.DEFAULT_CRUX_FORM_FACTORS);
+        expect(crux.CRUX_FORM_FACTORS).toBe(cruxHistory.CRUX_FORM_FACTORS);
+    });
+
+    it('rejects a push that would add a form factor to every later call', () => {
+        expect(() => crux.DEFAULT_CRUX_FORM_FACTORS.push('tablet')).toThrow(TypeError);
+        expect(crux.DEFAULT_CRUX_FORM_FACTORS).toEqual(['phone', 'desktop']);
+    });
+
+    it('still allows callers to extend a copy', () => {
+        expect([...crux.DEFAULT_CRUX_FORM_FACTORS, 'tablet']).toEqual(['phone', 'desktop', 'tablet']);
+    });
+});

lib/crux.js

@@ -18,6 +18,7 @@ const CRUX_API_URL = 'https://chromeuxreport.googleapis.com/v1/records:queryReco
  *   extractedAt: string
  * }} CruxReport
  *
+ * @typedef {chromeuxreport_v1.Schema$Metric} CruxMetric
  * @typedef {{ url: string, formFactor: CruxFormFactor }} CruxWorkItem
  * @typedef {{ url: string, formFactor: CruxFormFactor, data: CruxReport|null, noData: boolean, error: string|null }} CruxBatchResult
  * @typedef {{ url: string, formFactor: CruxFormFactor, outputPath: string|null, noData: boolean, error: string|null }} CruxBatchWriteResult

lib/index.js

@@ -1,3 +1,21 @@
+/**
+ * The types the README's "Key exported types" table promises from the package root. Re-declared
+ * here because lib/index.js is a value-only façade of lazy getters: without these, the
+ * documented `import type { CruxReport } from '@hugoer/web-perf-cli'` fails with TS2305.
+ *
+ * @typedef {import('./lab').LabReport} LabReport
+ * @typedef {import('./lab').LabPlanResult} LabPlanResult
+ * @typedef {import('./psi').PsiReport} PsiReport
+ * @typedef {import('./psi').PsiBatchResult} PsiBatchResult
+ * @typedef {import('./crux').CruxReport} CruxReport
+ * @typedef {import('./crux').CruxMetric} CruxMetric
+ * @typedef {import('./crux').CruxBatchResult} CruxBatchResult
+ * @typedef {import('./crux').CruxFormFactor} CruxFormFactor
+ * @typedef {import('./crux-history').CruxHistoryReport} CruxHistoryReport
+ * @typedef {import('./crux-history').CruxHistoryBatchResult} CruxHistoryBatchResult
+ * @typedef {import('./variance').RunSummary} RunSummary
+ */
+
 const lazy = (loader) => {
     let cached;
     return () => {

lib/lab.js

@@ -62,6 +62,15 @@ const CHROME_FLAGS = [
  *   declaring it required promised consumers a field the default path deletes.
  * @property {Record<string, LighthouseCategory>} categories
  * @property {Record<string, LighthouseAudit>} audits
+ * @property {{ benchmarkIndex?: number, hostUserAgent?: string, networkUserAgent?: string }} [environment]
+ *   Survives stripJsonProps, which drops only `i18n` and `timing`. buildRunSummary reads
+ *   `environment.benchmarkIndex` from it.
+ * @property {{ code: string, message?: string }} [runtimeError] - present when Lighthouse
+ *   resolved rather than threw: the page failed to load and the report carries no usable
+ *   metrics. runLabPlan treats a report with this set as a failed run.
+ * @property {{ formFactor?: 'desktop'|'mobile', [key: string]: unknown }} [configSettings]
+ *   Also survives stripJsonProps. cleanLabReport hoists formFactor out of it, because
+ *   Lighthouse 13 moved the field here from the report root.
  */
 
 /**
@@ -86,9 +95,9 @@ const CHROME_FLAGS = [
  * @property {(ctx: LabPlanContext) => void} [onRunStart]
  * @property {(ctx: LabPlanContext & { outputPath: string, report: LabReport }) => void} [onRunComplete]
  * @property {(ctx: LabPlanContext & { error: string, outputPath?: string }) => void} [onRunError]
- * @property {(ctx: { url: string, profile: string, summary: object, summaryPath: string }) => void} [onSummary]
+ * @property {(ctx: { url: string, profile: string, summary: import('./variance').RunSummary, summaryPath: string }) => void} [onSummary]
  *
- * @typedef {Object} LabPlanOptions
+ * @typedef {Object} LabPlanControls
  * @property {boolean} [continueOnError=false] - collect failures instead of aborting the plan
  * @property {boolean} [reuseBrowser=false] - share one Chrome across every run. Faster, but
  *   Lighthouse does not clear DNS caches or socket pools between runs, so later runs start
@@ -99,6 +108,32 @@ const CHROME_FLAGS = [
  * @property {(opts: object) => Promise<{ port: number, kill: () => Promise<void> }>} [_launch] - injectable Chrome launcher (tests)
  */
 
+/**
+ * What runLabToDisk takes: the audit options, plus the flag that writes an AI-friendly copy
+ * beside the raw report. `runNumber` is set by runLabPlan itself, not by callers.
+ * @typedef {LabAuditOptions & { clean?: boolean, runNumber?: number }} LabWriteOptions
+ */
+
+/**
+ * Plan-level controls plus the per-run options forwarded to every audit.
+ *
+ * runLabPlan destructures the controls and spreads the rest into each runLabToDisk call, so
+ * the two halves genuinely are one options object to a caller. Declaring only the controls
+ * made the published type reject `skipAudits`, `categories`, `blockedUrlPatterns`,
+ * `stripJsonProps`, `clean` and `silent` — every option bin/web-perf.js actually passes.
+ *
+ * `runNumber` and `port` are excluded because runLabPlan owns both, and a caller-supplied
+ * value survives to do damage rather than being overridden:
+ *
+ * - `runNumber` is only replaced when `repeats > 1`, so passing it to a single-run plan
+ *   stamps every report in the plan with the same `-runNN` suffix.
+ * - `port` is only replaced when the plan launched its own Chrome, so passing it with
+ *   `reuseBrowser: false` shares one browser across every run anyway — the position-dependent
+ *   scoring `reuseBrowser` warns about, with none of the warning.
+ *
+ * @typedef {LabPlanControls & Omit<LabWriteOptions, 'runNumber' | 'port'>} LabPlanOptions
+ */
+
 function buildLighthouseConfig(labOptions, profileSettings = {}) {
     const rawSkipAudits = labOptions.skipAudits || DEFAULT_SKIP_AUDITS;
     const disableFullPageScreenshot = rawSkipAudits.includes('full-page-screenshot');
@@ -194,7 +229,7 @@ function buildRunSuffix(labOptions) {
  * `runLab` wraps this to keep its published `Promise<string>` signature; `runLabPlan` uses
  * it directly so summaries can read scores without re-parsing the file it just wrote.
  * @param {string} url
- * @param {object} [labOptions]
+ * @param {LabWriteOptions} [labOptions]
  * @returns {Promise<{ outputPath: string, data: LabReport }>}
  */
 async function runLabToDisk(url, labOptions = {}) {

package.json

@@ -15,61 +15,62 @@
   },
   "exports": {
     ".": {
+      "types": "./types/lib/index.d.ts",
       "require": "./lib/index.js",
-      "import": "./lib/index.js",
-      "types": "./types/lib/index.d.ts"
+      "import": "./lib/index.js"
     },
     "./lab": {
+      "types": "./types/lib/lab.d.ts",
       "require": "./lib/lab.js",
-      "import": "./lib/lab.js",
-      "types": "./types/lib/lab.d.ts"
+      "import": "./lib/lab.js"
     },
     "./psi": {
+      "types": "./types/lib/psi.d.ts",
       "require": "./lib/psi.js",
-      "import": "./lib/psi.js",
-      "types": "./types/lib/psi.d.ts"
+      "import": "./lib/psi.js"
     },
     "./crux": {
+      "types": "./types/lib/crux.d.ts",
       "require": "./lib/crux.js",
-      "import": "./lib/crux.js",
-      "types": "./types/lib/crux.d.ts"
+      "import": "./lib/crux.js"
     },
     "./crux-history": {
+      "types": "./types/lib/crux-history.d.ts",
       "require": "./lib/crux-history.js",
-      "import": "./lib/crux-history.js",
-      "types": "./types/lib/crux-history.d.ts"
+      "import": "./lib/crux-history.js"
     },
     "./utils": {
+      "types": "./types/lib/utils.d.ts",
       "require": "./lib/utils.js",
-      "import": "./lib/utils.js",
-      "types": "./types/lib/utils.d.ts"
+      "import": "./lib/utils.js"
     },
     "./profiles": {
+      "types": "./types/lib/profiles.d.ts",
       "require": "./lib/profiles.js",
-      "import": "./lib/profiles.js",
-      "types": "./types/lib/profiles.d.ts"
+      "import": "./lib/profiles.js"
     },
     "./links": {
+      "types": "./types/lib/links.d.ts",
       "require": "./lib/links.js",
-      "import": "./lib/links.js",
-      "types": "./types/lib/links.d.ts"
+      "import": "./lib/links.js"
     },
     "./sitemap": {
+      "types": "./types/lib/sitemap.d.ts",
       "require": "./lib/sitemap.js",
-      "import": "./lib/sitemap.js",
-      "types": "./types/lib/sitemap.d.ts"
+      "import": "./lib/sitemap.js"
     },
     "./variance": {
+      "types": "./types/lib/variance.d.ts",
       "require": "./lib/variance.js",
-      "import": "./lib/variance.js",
-      "types": "./types/lib/variance.d.ts"
+      "import": "./lib/variance.js"
     }
   },
   "scripts": {
     "start": "node bin/web-perf.js",
     "lint": "eslint . --fix",
     "test": "vitest run",
-    "generate-types": "tsc --project tsconfig.types.json"
+    "generate-types": "tsc --project tsconfig.types.json",
+    "check-types": "tsc --project type-tests/tsconfig.json"
   },
   "dependencies": {
     "chalk": "^6.0.0",