From 037be15cf200f27049862cdaf63c3176e28175b4 Mon Sep 17 00:00:00 2001 From: os-sam Date: Sat, 5 Sep 2026 19:15:47 +0000 Subject: [PATCH 1/3] feat(tooling): gate doc comments that prescribe a spelling a shared reader owns (objectui#7652) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A JSDoc `@example` on an exported hook is copied by call sites. When the ruling it encoded moves, the prose stays and seeds every later copy — so fixing the call sites without fixing the doc comment re-seeds them. That class cost two cards and three copied call sites (objectui#7627, objectui#7638), and objectui#7617's `check-spec-symbol-derivation` was credited with covering it twice while its rule 4 judges `@objectstack/spec` citations at member granularity and says nothing about prose prescribing a local spelling. The gate fires on four conditions at once: the `@example` calls the symbol it documents, a real in-repo call site fills the same argument slot by calling an exported single-`return` reader, the example does not, and what the example writes there is that reader's own return expression or one of the rungs it resolves between. That last condition is what keeps it off the literals and placeholders an example legitimately carries. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3 --- .github/workflows/ci.yml | 18 + content/docs/guide/ci-cd-pipeline.md | 2 +- package.json | 1 + .../check-doc-example-shared-reader.test.ts | 376 ++++++++++++ scripts/check-doc-example-shared-reader.mjs | 544 ++++++++++++++++++ 5 files changed, 940 insertions(+), 1 deletion(-) create mode 100644 scripts/__tests__/check-doc-example-shared-reader.test.ts create mode 100644 scripts/check-doc-example-shared-reader.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 62c64b141..c3a9d006d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -292,6 +292,24 @@ jobs: if: steps.relevant.outputs.should_run == 'true' run: pnpm check:unreferenced-sources + # A JSDoc `@example` on an exported hook is not decoration: call sites copy it. + # When the ruling the example encoded moves, the prose stays, and every later + # copy is seeded from the prose rather than from the code — so fixing the call + # sites without fixing the doc comment re-seeds them. That class cost two cards + # and three copied call sites (objectui#7627, objectui#7638) with nothing in CI + # able to see it; objectui#7617's `check-spec-symbol-derivation` was cited as + # covering it twice and does not — its rule 4 judges `@objectstack/spec` + # citations at member granularity and has nothing to say about prose that + # prescribes a LOCAL spelling. + # + # This step fails only when a documented symbol's own `@example` hand-spells + # what its real call sites obtain by CALLING a shared reader. Parses sources + # with `typescript` through the same scanner as the two steps above, so it + # needs the install and nothing built. + - name: Verify no doc comment prescribes a spelling a shared reader owns + if: steps.relevant.outputs.should_run == 'true' + run: pnpm check:doc-example-readers + # A build tsconfig that excludes tooling by FILE NAME (`*.test.ts`) stops # the files that happen to be named that way and nothing else. The first # shared helper added to a `__tests__/` directory is then a program input, diff --git a/content/docs/guide/ci-cd-pipeline.md b/content/docs/guide/ci-cd-pipeline.md index 21c1195a1..3c6733f74 100644 --- a/content/docs/guide/ci-cd-pipeline.md +++ b/content/docs/guide/ci-cd-pipeline.md @@ -208,7 +208,7 @@ it green — which is how two of `type-check`'s gates came to be missing from th | Job key | Appears as | What it runs | When | |---|---|---|---| | `changeset-check` | Changeset Fixed Group Check | `scripts/check-changeset-fixed.mjs` — every workspace package must be in the changeset `fixed` group or explicitly ignored. It checks group *membership*; it does **not** check whether the PR added a changeset. | Every run | -| `type-check` | Type Check | `scripts/check-type-check-coverage.mjs`, then `pnpm check:phantom-deps`, then `pnpm check:self-import`, then `pnpm check:unreferenced-sources`, then `pnpm check:published-tsconfig-exclude`, then `pnpm check:side-effects-array`, then `pnpm check:element-data-source-declaration`, then `pnpm check:esm-specifiers`, then `pnpm check:spec-symbols`, then `pnpm check:action-forward-parity`, then `pnpm check:designer-field-key-parity`, then `pnpm check:icon-record-names`, then `pnpm check:i18n-keys`, then `pnpm check:i18n-drift`, then `pnpm type-check:scripts`, then `pnpm type-check`, then `pnpm type-check:vitest-setup`. The coverage guard runs first because turbo silently skips packages that have no `type-check` script, so a package without one would otherwise read as passing (#2911). `pnpm check:phantom-deps` fails when a released package imports a bare specifier its own `package.json` does not declare — a *phantom dependency*, invisible locally because the workspace root's `devDependencies` sit on the upward resolution path from every package directory and on no consumer's, so `require.resolve('react', { paths: ['packages/core/src'] })` succeeds while `@object-ui/core` declares react in no field at all ([#4394](https://github.com/objectstack-ai/objectui/issues/4394)). `pnpm check:self-import` runs next because it reuses that gate's parser: it fails when a file inside a package names its OWN package, a specifier that resolves through the package's `exports` map to `dist/` while `type-check` waits on `^build` — the *dependencies'* builds, never the package's own — so on a cold cache the declarations do not exist yet and the file fails with `TS2307`. Locally it is always green, because every local workflow builds before it type-checks and leaves a `dist/` behind; PR #4789's first run was red on exactly one such line ([#4801](https://github.com/objectstack-ai/objectui/issues/4801)). `pnpm check:unreferenced-sources` runs next, reusing the same parser again: it fails when a covered package ships a source file that nothing reaches — not the package's declared entry, and not its build config. Until [#7515](https://github.com/objectstack-ai/objectui/issues/7515) no gate here could see one: `check-dist-completeness` asks whether `dist/` holds what `tsc` emits, `check-readme-exports` compares documented exports against shipped ones, and a file that is in the tarball while being reachable from nothing is outside both — so the detection mechanism was a human reading unrelated code, which is how both instances found in one week were found ([#7319](https://github.com/objectstack-ai/objectui/issues/7319), [#7397](https://github.com/objectstack-ai/objectui/issues/7397)). The hazard is not the bytes: the file #7319 removed carried the same export name as a live engine one package over and evaluated no predicate, so name-completion alone could have wired a silently wrong renderer into a published package. Reachability has TWO roots, and the second is the whole difficulty — `packages/components` reaches its two `use-sync-external-store` shims only through `vite.config.ts` `resolve.alias` entries whose importer is a bundled dependency no source file names, so a walk that skips that leg reports exactly those two live files as dead on its first run, and a gate that cries wolf gets switched off rather than fixed. Scope is DECLARED per package in `COVERED_PACKAGES` and the uncovered remainder is printed as a count derived from the workspace on every run, because the alias mechanisms differ per package and a gate that covers one package correctly beats one that covers forty with false positives. An alias expression it cannot evaluate is a FINDING rather than a skip, since skipping one would make it accuse whatever file that alias points at. `pnpm check:published-tsconfig-exclude` follows, config reads only: it fails when a published package's build `tsconfig.json` excludes tooling by FILE NAME (`*.test.ts`) without also excluding the tooling DIRECTORIES (`**/__tests__/**` and its two siblings, derived from `TOOLING_FILE` rather than retyped). A name-only exclude stops the files that happen to be named that way and nothing else, so the first shared helper added to a `__tests__/` directory becomes a program input and an emitting program writes it into the published `dist` — three times so far, each found by a human and never by a gate ([#4006](https://github.com/objectstack-ai/objectui/issues/4006), [#4836](https://github.com/objectstack-ai/objectui/issues/4836), [#6943](https://github.com/objectstack-ai/objectui/issues/6943), the third in the same package as the first). [#7212](https://github.com/objectstack-ai/objectui/issues/7212) measured the standing exposure — 29 published packages carrying the name form with ZERO offending files, green because nobody had added such a helper yet — and the gate landed together with their conversion so `main` was green on merge. It reads `exclude` arrays and nothing else: no build, no artifact, no emit model, which is the narrower scope that keeps it clear of the modelling [#4846](https://github.com/objectstack-ai/objectui/issues/4846) declined for the artifact-level gate. Six published packages are named carve-outs, each re-proving its own reason on every run: `cli`, `create-plugin` and `data-objectstack` emit from a `tsup` entry graph, `plugin-charts` keeps its tooling exclude in the `dts()` options, and `console` and `runner` are Vite applications with `noEmit: true` and no `dts()` plugin. `pnpm check:side-effects-array` runs next, sources only and no build: it fails when a package's `sideEffects` ARRAY and its module bodies disagree in either direction — a module that registers something at load time and is not named (a bundler drops it, and the registration is gone from a *consumer's* app with no error, no warning and exit 0), or a name whose module no longer registers anything. `@object-ui/app-shell` declares such an array because both simpler answers are measurably wrong for it: omitting the field makes the whole package unshakeable, and `"sideEffects": false` silently drops three live SDUI widget registrations to zero chunks ([#6535](https://github.com/objectstack-ai/objectui/issues/6535), [#6683](https://github.com/objectstack-ai/objectui/issues/6683)). The enumeration is re-derived from the module bodies on every run rather than listed, so there is no second copy to rot. The artifact half of the same contract — do those registrations survive a real bundler — cannot run in this job at all: it needs a built console, so it lives in the SDUI registration pin step of `performance-budget.yml`. `pnpm check:element-data-source-declaration` runs next, sources only and no build: it fails when a source that consumes `ElementDataSourceGate` does not also pass through `elementDataSourceBlock()`, the seam that declares the `dataSource` key the gate reads. A block that wraps the gate off-seam publishes an authoring surface missing the one key its own runtime honours, and the html tier reports that key with the same `unknown-prop` warning it gives the spellings that do nothing ([#6678](https://github.com/objectstack-ai/objectui/issues/6678)). `pnpm check:esm-specifiers` follows it for the same reason — sources only, no build: it fails when a published package whose build preserves import specifiers (a bare emitting `tsc`, which never rewrites them) writes a relative specifier with no file extension. Node's ESM resolver does not extension-search relative specifiers, so such a specifier makes the published entry unloadable outside a bundler; `@object-ui/react`'s entry died with `ERR_MODULE_NOT_FOUND` while every bundler-based consumer, the whole test suite and CI stayed green ([#4538](https://github.com/objectstack-ai/objectui/issues/4538)). The half that actually *imports* each built entry needs a full build and runs in `node-esm-load-gate.yml`. `pnpm check:action-forward-parity` fails when an action renderer's forward whitelist drops a key the action runtime reads — the class that shipped six times one key at a time, each time green, because the key parses and publishes while the payload is dropped one hop before the runner ([#4050](https://github.com/objectstack-ai/objectui/issues/4050)). `pnpm check:designer-field-key-parity` fails when one of the field designers' statically declared payload shapes (`FieldMetadataPayload`, `ServerFieldSchema`, `DesignerFieldDefinition`) declares a key the installed `@objectstack/spec` `FieldSchema` refuses by NAME. Such a key makes `PUT /api/v1/meta/object/:name` return a hard 422 `INVALID_METADATA` that blocks *every subsequent save* of that object, and the author cannot tell from the designer UI which key did it — the class had been filed three times, each closed with a per-key tombstone written after the instance was found in production, with nothing detecting the next one ([#4644](https://github.com/objectstack-ai/objectui/issues/4644) `indexed`, [#4687](https://github.com/objectstack-ai/objectui/issues/4687) `distance_metric`, [#4676](https://github.com/objectstack-ai/objectui/issues/4676) `placeholder`, gated by [#5761](https://github.com/objectstack-ai/objectui/issues/5761)). It reads the accept set off the schema itself rather than from a list, and it covers a deliberately documented *subset* of the write path: a key that reaches the payload only through a `patchDef` spread or an index signature is outside its reach, and the boundary is stated in the script's own docblock. Its draft-I/O half — the `readFields`/`writeFields` round-trip, which has no declared shape to read — runs in the test suite as `object-fields-io.spec-keys.test.ts`. Same placement rationale as the gates around it: it parses the sources with `typescript` and imports the installed spec, so it needs the install and nothing built. `pnpm check:icon-record-names` fails when an authored icon NAME that reaches a resolver reading lucide's runtime `icons` record is not a live key of that record. lucide retires a spelling by dropping it from that record while keeping it as a deprecated named export, so the retired name still imports, still type-checks and still renders wherever it is used as a *component* — `Edit === SquarePen` is true — and resolves to nothing wherever it is used as a *string*: nothing goes red in either direction, which is why the class was repaired twice in two packages before anyone gated it ([#5586](https://github.com/objectstack-ai/objectui/issues/5586), [#5622](https://github.com/objectstack-ai/objectui/issues/5622), [#5633](https://github.com/objectstack-ai/objectui/issues/5633)). It carries no list of retired spellings — the record itself is the judgement — and it re-discovers the resolver population from source on every run, which is how its first pass found four record-reading resolvers nobody had catalogued. It sits here because it parses the sources with `typescript` and reads the installed lucide: the install, and nothing built. The two locale gates sit in the middle because both parse the sources with `typescript`: they need the install and nothing built. `pnpm check:i18n-keys` fails when a `t()` call site asks for a key the `en` pack does not define ([#3530](https://github.com/objectstack-ai/objectui/issues/3530)); `pnpm check:i18n-drift` fails when a change to an `en` string is not accompanied by the nine translation packs ([#3650](https://github.com/objectstack-ai/objectui/issues/3650)), and it is why this job's checkout sets `fetch-depth: 0` — it diffs against the merge base, which a depth-1 clone cannot resolve. `pnpm type-check:scripts` (`tsconfig.scripts.json`) covers `scripts/**/*.ts`, which `pnpm type-check` cannot reach at all — `scripts/` has no package.json, so turbo never walks it, and the coverage guard decides coverage per *package*. Until [#3494](https://github.com/objectstack-ai/objectui/issues/3494) that left the pin tests in `scripts/__tests__/` — including the one pinning this very page — compiled by nothing. `pnpm type-check:vitest-setup` (`tsconfig.vitest-setup.json`) closes the same gap for the four repo-root `vitest.setup.*` files, uncovered until [#3515](https://github.com/objectstack-ai/objectui/issues/3515); it runs *last*, after `pnpm type-check`, because `vitest.setup.dom.tsx` side-effect-imports four `@object-ui/*` packages and resolves them through the declarations that turbo's `^build` produces. | Every run; on a PR the steps short-circuit when only ignored paths changed | +| `type-check` | Type Check | `scripts/check-type-check-coverage.mjs`, then `pnpm check:phantom-deps`, then `pnpm check:self-import`, then `pnpm check:unreferenced-sources`, then `pnpm check:doc-example-readers`, then `pnpm check:published-tsconfig-exclude`, then `pnpm check:side-effects-array`, then `pnpm check:element-data-source-declaration`, then `pnpm check:esm-specifiers`, then `pnpm check:spec-symbols`, then `pnpm check:action-forward-parity`, then `pnpm check:designer-field-key-parity`, then `pnpm check:icon-record-names`, then `pnpm check:i18n-keys`, then `pnpm check:i18n-drift`, then `pnpm type-check:scripts`, then `pnpm type-check`, then `pnpm type-check:vitest-setup`. The coverage guard runs first because turbo silently skips packages that have no `type-check` script, so a package without one would otherwise read as passing (#2911). `pnpm check:phantom-deps` fails when a released package imports a bare specifier its own `package.json` does not declare — a *phantom dependency*, invisible locally because the workspace root's `devDependencies` sit on the upward resolution path from every package directory and on no consumer's, so `require.resolve('react', { paths: ['packages/core/src'] })` succeeds while `@object-ui/core` declares react in no field at all ([#4394](https://github.com/objectstack-ai/objectui/issues/4394)). `pnpm check:self-import` runs next because it reuses that gate's parser: it fails when a file inside a package names its OWN package, a specifier that resolves through the package's `exports` map to `dist/` while `type-check` waits on `^build` — the *dependencies'* builds, never the package's own — so on a cold cache the declarations do not exist yet and the file fails with `TS2307`. Locally it is always green, because every local workflow builds before it type-checks and leaves a `dist/` behind; PR #4789's first run was red on exactly one such line ([#4801](https://github.com/objectstack-ai/objectui/issues/4801)). `pnpm check:unreferenced-sources` runs next, reusing the same parser again: it fails when a covered package ships a source file that nothing reaches — not the package's declared entry, and not its build config. Until [#7515](https://github.com/objectstack-ai/objectui/issues/7515) no gate here could see one: `check-dist-completeness` asks whether `dist/` holds what `tsc` emits, `check-readme-exports` compares documented exports against shipped ones, and a file that is in the tarball while being reachable from nothing is outside both — so the detection mechanism was a human reading unrelated code, which is how both instances found in one week were found ([#7319](https://github.com/objectstack-ai/objectui/issues/7319), [#7397](https://github.com/objectstack-ai/objectui/issues/7397)). The hazard is not the bytes: the file #7319 removed carried the same export name as a live engine one package over and evaluated no predicate, so name-completion alone could have wired a silently wrong renderer into a published package. Reachability has TWO roots, and the second is the whole difficulty — `packages/components` reaches its two `use-sync-external-store` shims only through `vite.config.ts` `resolve.alias` entries whose importer is a bundled dependency no source file names, so a walk that skips that leg reports exactly those two live files as dead on its first run, and a gate that cries wolf gets switched off rather than fixed. Scope is DECLARED per package in `COVERED_PACKAGES` and the uncovered remainder is printed as a count derived from the workspace on every run, because the alias mechanisms differ per package and a gate that covers one package correctly beats one that covers forty with false positives. An alias expression it cannot evaluate is a FINDING rather than a skip, since skipping one would make it accuse whatever file that alias points at. `pnpm check:doc-example-readers` runs next, on the same parser again: it fails when an exported symbol's own JSDoc `@example` hand-spells a resolution that its REAL call sites obtain by calling a shared reader. A doc comment is what the next call site is copied from, so prose that outlives the ruling it encoded re-seeds every later copy — measured at two cards and three copied call sites ([#7627](https://github.com/objectstack-ai/objectui/issues/7627), [#7638](https://github.com/objectstack-ai/objectui/issues/7638)), both closed by pointing the prose at `resolveRecordSourceObjectName`. Nothing here could see either one, and `check-spec-symbol-derivation` was credited with the class twice — in #7638's card body and then in the dispatch that repeated it — while its rule 4 judges `@objectstack/spec` citations at member granularity and says nothing about prose prescribing a LOCAL spelling ([#7652](https://github.com/objectstack-ai/objectui/issues/7652)). It fires on four conditions at once — the example calls the symbol it documents, a real call site fills the same argument slot by calling an exported single-`return` reader, the example does not, and what the example writes there is that reader's own return expression or one of the rungs it resolves between — which is what keeps it off the literals and placeholders an example legitimately carries. It does NOT judge whether a prescribed spelling is correct: on the day either card was filed the prose and every copy of it agreed, and no gate reading only the tree can know a ruling. What it catches is the state right after, when the call sites move and the prose does not. `pnpm check:published-tsconfig-exclude` follows, config reads only: it fails when a published package's build `tsconfig.json` excludes tooling by FILE NAME (`*.test.ts`) without also excluding the tooling DIRECTORIES (`**/__tests__/**` and its two siblings, derived from `TOOLING_FILE` rather than retyped). A name-only exclude stops the files that happen to be named that way and nothing else, so the first shared helper added to a `__tests__/` directory becomes a program input and an emitting program writes it into the published `dist` — three times so far, each found by a human and never by a gate ([#4006](https://github.com/objectstack-ai/objectui/issues/4006), [#4836](https://github.com/objectstack-ai/objectui/issues/4836), [#6943](https://github.com/objectstack-ai/objectui/issues/6943), the third in the same package as the first). [#7212](https://github.com/objectstack-ai/objectui/issues/7212) measured the standing exposure — 29 published packages carrying the name form with ZERO offending files, green because nobody had added such a helper yet — and the gate landed together with their conversion so `main` was green on merge. It reads `exclude` arrays and nothing else: no build, no artifact, no emit model, which is the narrower scope that keeps it clear of the modelling [#4846](https://github.com/objectstack-ai/objectui/issues/4846) declined for the artifact-level gate. Six published packages are named carve-outs, each re-proving its own reason on every run: `cli`, `create-plugin` and `data-objectstack` emit from a `tsup` entry graph, `plugin-charts` keeps its tooling exclude in the `dts()` options, and `console` and `runner` are Vite applications with `noEmit: true` and no `dts()` plugin. `pnpm check:side-effects-array` runs next, sources only and no build: it fails when a package's `sideEffects` ARRAY and its module bodies disagree in either direction — a module that registers something at load time and is not named (a bundler drops it, and the registration is gone from a *consumer's* app with no error, no warning and exit 0), or a name whose module no longer registers anything. `@object-ui/app-shell` declares such an array because both simpler answers are measurably wrong for it: omitting the field makes the whole package unshakeable, and `"sideEffects": false` silently drops three live SDUI widget registrations to zero chunks ([#6535](https://github.com/objectstack-ai/objectui/issues/6535), [#6683](https://github.com/objectstack-ai/objectui/issues/6683)). The enumeration is re-derived from the module bodies on every run rather than listed, so there is no second copy to rot. The artifact half of the same contract — do those registrations survive a real bundler — cannot run in this job at all: it needs a built console, so it lives in the SDUI registration pin step of `performance-budget.yml`. `pnpm check:element-data-source-declaration` runs next, sources only and no build: it fails when a source that consumes `ElementDataSourceGate` does not also pass through `elementDataSourceBlock()`, the seam that declares the `dataSource` key the gate reads. A block that wraps the gate off-seam publishes an authoring surface missing the one key its own runtime honours, and the html tier reports that key with the same `unknown-prop` warning it gives the spellings that do nothing ([#6678](https://github.com/objectstack-ai/objectui/issues/6678)). `pnpm check:esm-specifiers` follows it for the same reason — sources only, no build: it fails when a published package whose build preserves import specifiers (a bare emitting `tsc`, which never rewrites them) writes a relative specifier with no file extension. Node's ESM resolver does not extension-search relative specifiers, so such a specifier makes the published entry unloadable outside a bundler; `@object-ui/react`'s entry died with `ERR_MODULE_NOT_FOUND` while every bundler-based consumer, the whole test suite and CI stayed green ([#4538](https://github.com/objectstack-ai/objectui/issues/4538)). The half that actually *imports* each built entry needs a full build and runs in `node-esm-load-gate.yml`. `pnpm check:action-forward-parity` fails when an action renderer's forward whitelist drops a key the action runtime reads — the class that shipped six times one key at a time, each time green, because the key parses and publishes while the payload is dropped one hop before the runner ([#4050](https://github.com/objectstack-ai/objectui/issues/4050)). `pnpm check:designer-field-key-parity` fails when one of the field designers' statically declared payload shapes (`FieldMetadataPayload`, `ServerFieldSchema`, `DesignerFieldDefinition`) declares a key the installed `@objectstack/spec` `FieldSchema` refuses by NAME. Such a key makes `PUT /api/v1/meta/object/:name` return a hard 422 `INVALID_METADATA` that blocks *every subsequent save* of that object, and the author cannot tell from the designer UI which key did it — the class had been filed three times, each closed with a per-key tombstone written after the instance was found in production, with nothing detecting the next one ([#4644](https://github.com/objectstack-ai/objectui/issues/4644) `indexed`, [#4687](https://github.com/objectstack-ai/objectui/issues/4687) `distance_metric`, [#4676](https://github.com/objectstack-ai/objectui/issues/4676) `placeholder`, gated by [#5761](https://github.com/objectstack-ai/objectui/issues/5761)). It reads the accept set off the schema itself rather than from a list, and it covers a deliberately documented *subset* of the write path: a key that reaches the payload only through a `patchDef` spread or an index signature is outside its reach, and the boundary is stated in the script's own docblock. Its draft-I/O half — the `readFields`/`writeFields` round-trip, which has no declared shape to read — runs in the test suite as `object-fields-io.spec-keys.test.ts`. Same placement rationale as the gates around it: it parses the sources with `typescript` and imports the installed spec, so it needs the install and nothing built. `pnpm check:icon-record-names` fails when an authored icon NAME that reaches a resolver reading lucide's runtime `icons` record is not a live key of that record. lucide retires a spelling by dropping it from that record while keeping it as a deprecated named export, so the retired name still imports, still type-checks and still renders wherever it is used as a *component* — `Edit === SquarePen` is true — and resolves to nothing wherever it is used as a *string*: nothing goes red in either direction, which is why the class was repaired twice in two packages before anyone gated it ([#5586](https://github.com/objectstack-ai/objectui/issues/5586), [#5622](https://github.com/objectstack-ai/objectui/issues/5622), [#5633](https://github.com/objectstack-ai/objectui/issues/5633)). It carries no list of retired spellings — the record itself is the judgement — and it re-discovers the resolver population from source on every run, which is how its first pass found four record-reading resolvers nobody had catalogued. It sits here because it parses the sources with `typescript` and reads the installed lucide: the install, and nothing built. The two locale gates sit in the middle because both parse the sources with `typescript`: they need the install and nothing built. `pnpm check:i18n-keys` fails when a `t()` call site asks for a key the `en` pack does not define ([#3530](https://github.com/objectstack-ai/objectui/issues/3530)); `pnpm check:i18n-drift` fails when a change to an `en` string is not accompanied by the nine translation packs ([#3650](https://github.com/objectstack-ai/objectui/issues/3650)), and it is why this job's checkout sets `fetch-depth: 0` — it diffs against the merge base, which a depth-1 clone cannot resolve. `pnpm type-check:scripts` (`tsconfig.scripts.json`) covers `scripts/**/*.ts`, which `pnpm type-check` cannot reach at all — `scripts/` has no package.json, so turbo never walks it, and the coverage guard decides coverage per *package*. Until [#3494](https://github.com/objectstack-ai/objectui/issues/3494) that left the pin tests in `scripts/__tests__/` — including the one pinning this very page — compiled by nothing. `pnpm type-check:vitest-setup` (`tsconfig.vitest-setup.json`) closes the same gap for the four repo-root `vitest.setup.*` files, uncovered until [#3515](https://github.com/objectstack-ai/objectui/issues/3515); it runs *last*, after `pnpm type-check`, because `vitest.setup.dom.tsx` side-effect-imports four `@object-ui/*` packages and resolves them through the declarations that turbo's `^build` produces. | Every run; on a PR the steps short-circuit when only ignored paths changed | | `test` | Test (shard N/4) | `pnpm test --shard=N/4` across a 4-runner matrix with `fail-fast: false`, so every shard reports its own failures. No coverage instrumentation — v8 adds 40–100% overhead. Then, **on shard 1 only**, `pnpm test:dist` — the built-artifact lane ([#7183](https://github.com/objectstack-ai/objectui/issues/7183)). It delegates to a turbo task scoped to the one package that holds built-artifact pins; that task depends on the package's OWN build (`dependsOn: ["build"]`, not `^build`), so the bundle exists before the pins read it, and then runs the `dist` vitest project, whose pins import a package's BUILT bundle instead of its `src` — a claim the source-aliased suite above is structurally unable to make, since the root config aliases every workspace package to `src`. It is deliberately not sharded and not repeated on the other three runners: the lane is a handful of files, and running it on all four would pay for the same build four times. | Pull requests and merge-queue builds (everything but `push`); steps short-circuit on a PR that changed only ignored paths | | `test-coverage` | Test (coverage shard N/4) | `pnpm test:coverage --reporter=blob --shard=N/4` across a 4-runner matrix with `fail-fast: false`. Each shard writes `.vitest-reports/blob-N-4.json` — raw coverage and test results in one file — and uploads it as an artifact even when the shard is red, which is what makes a failing coverage run diagnosable at all (vitest deletes `coverage/` on a red run unless `coverage.reportOnFailure` is set, [#5402](https://github.com/objectstack-ai/objectui/issues/5402)). The configured coverage thresholds are neutralised on the shard legs, because a quarter of the suite judged against a whole-suite threshold is not a defect signal; they are enforced once, on the merged report, by the job below ([#5403](https://github.com/objectstack-ai/objectui/issues/5403)). | **Push only** | | `coverage-report` | Test (coverage) | Downloads the four blob reports, refuses to continue unless all four arrived, merges them with `pnpm test:coverage --merge-reports` into one complete report — which is where the configured coverage thresholds are enforced, over the whole merged map, the shard legs having overridden them to zero — and publishes that report as the `coverage-report` artifact (kept 7 days, the same as the blobs it is derived from). Its last step runs on every path and states the outcome: the job is **red, with an error annotation**, whenever the gate did not run for the commit — before [#5403](https://github.com/objectstack-ai/objectui/issues/5403) the final step carried the implicit `success()` and was silently skipped by 311 of 373 coverage jobs, which is how four days of a 100%-failing coverage job went unnoticed. A breach of the thresholds is reported *separately* from a lane that never delivered, because the two call for opposite actions. ⛔ It never merges a report from fewer than four shards: a wrong coverage number is worse than a missing one. The Codecov upload this job used to carry was retired by [#5436](https://github.com/objectstack-ai/objectui/issues/5436) — `CODECOV_TOKEN` was never set, so it failed on every push; the trend dashboard and PR coverage comments are gone with it, the gate is not. | **Push only** | diff --git a/package.json b/package.json index a20cc9c46..4d544cf30 100644 --- a/package.json +++ b/package.json @@ -76,6 +76,7 @@ "check:shell-escape-residue": "node scripts/check-shell-escape-residue.mjs", "check:readme-exports": "node scripts/check-readme-exports.mjs", "check:unreferenced-sources": "node scripts/check-unreferenced-sources.mjs", + "check:doc-example-readers": "node scripts/check-doc-example-shared-reader.mjs", "cli": "node packages/cli/dist/cli.js", "objectui": "node packages/cli/dist/cli.js", "create-plugin": "node packages/create-plugin/dist/index.js", diff --git a/scripts/__tests__/check-doc-example-shared-reader.test.ts b/scripts/__tests__/check-doc-example-shared-reader.test.ts new file mode 100644 index 000000000..9d47118cd --- /dev/null +++ b/scripts/__tests__/check-doc-example-shared-reader.test.ts @@ -0,0 +1,376 @@ +import { afterAll, describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import ts from 'typescript'; + +import { + KNOWN_HAND_SPELLINGS, + analyze, + canonical, + canonicalContains, + exampleFences, + inline, + rungsOf, +} from '../check-doc-example-shared-reader.mjs'; + +/** + * objectui#7652 — a doc comment must not prescribe a call-site spelling that a + * published shared reader already owns. + * + * A JSDoc `@example` is copied. When the ruling it encoded moves, the prose stays + * and seeds every later copy, so fixing the call sites without fixing the prose + * re-seeds them. That cost two cards and three copied call sites + * (objectui#7627 `useSettledSchema`, objectui#7638 `useNavigationOverlay`), and + * objectui#7617's `check-spec-symbol-derivation` was credited with covering the + * class twice — in #7638's card body, then in the dispatch that repeated it — + * while its rule 4 judges `@objectstack/spec` citations at MEMBER granularity and + * has nothing to say about prose prescribing a local spelling. + * + * What this file pins, in the order the gate can go wrong: + * + * 1. **A zero from this gate is a reading, not a dead instrument.** The lit + * control plants an instance in the very doc block whose zero the gate + * reports and requires exit-1 behaviour naming that file — and then a control + * ON that control: a NEAR-MISS plant, an ordinary example spelling that is + * not a rung of the reader, must stay green. Without the second half "the + * plant reddens it" only proves the gate reacts to edits. + * 2. **The historical instance, on the real tree that carried it.** The + * `useNavigationOverlay` shape as it stood between PR #7637 and PR #7648, + * rebuilt as a fixture: the reader exists, a call site delegates to it, the + * doc comment still writes the rung. + * 3. **The narrowing holds in both directions.** A literal, a placeholder and a + * locally-named variable in an example are all legal; only the reader's own + * return expression or one of its rungs is not. + * 4. **The trigger is a real call site, not a helper's existence.** A reader + * nobody calls says nothing about any doc comment. + * 5. **The scan cannot collapse quietly** — a green over an empty population is + * the failure this whole gate family exists to prevent. + * 6. **This repository is green**, and the exemption ledger stays empty. + * 7. **The gate is wired** where the sibling parse-based gates run, and the page + * that inventories them names it. + */ +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); + +const fixtures: string[] = []; +afterAll(() => { + for (const dir of fixtures) fs.rmSync(dir, { recursive: true, force: true }); +}); + +/** + * A throwaway tree in the shape the gate walks: `packages//src/`. + * Written to disk rather than parsed from strings because the population walk — + * which directories are read, which files are skipped as tooling — is half of + * what can go wrong, and a string-fed test would never exercise it. + */ +function tree(label: string, files: Record): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), `doc-example-${label}-`)); + fixtures.push(root); + for (const [rel, contents] of Object.entries(files)) { + const full = path.join(root, rel); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, contents); + } + return root; +} + +/** The shared reader, verbatim in shape from `packages/core/src/utils/record-source.ts`. */ +const READER = ` +export function resolveRecordSourceObjectName( + schema: { objectName?: string } | null | undefined, + dataConfig: { provider?: string; object?: string } | null | undefined, +): string | undefined { + return dataConfig?.provider === 'object' ? dataConfig.object : schema?.objectName; +} +`; + +/** The hook, with whatever its `@example` prescribes for `objectName`. */ +function hook(exampleSpelling: string): string { + return ` +/** + * Hook for NavigationConfig-driven navigation overlay. + * + * @example + * \`\`\`tsx + * const nav = useNavigationOverlay({ + * navigation: schema.navigation, + * objectName: ${exampleSpelling}, + * }); + * \`\`\` + */ +export function useNavigationOverlay(options: { navigation?: unknown; objectName?: string }) { + return options; +} +`; +} + +/** A caller that resolves the slot through the shared reader, as the fix left them. */ +const DELEGATING_CALLER = ` +import { resolveRecordSourceObjectName } from '@object-ui/core'; +import { useNavigationOverlay } from '@object-ui/react'; + +export function ObjectTree(props: any) { + const dataConfig = props.dataConfig; + const schema = props.schema; + const navigation = useNavigationOverlay({ + navigation: schema.navigation, + objectName: resolveRecordSourceObjectName(schema, dataConfig), + }); + return navigation; +} +`; + +function reseedingTree(exampleSpelling: string, label: string): string { + return tree(label, { + 'packages/core/src/utils/record-source.ts': READER, + 'packages/react/src/hooks/useNavigationOverlay.ts': hook(exampleSpelling), + 'packages/plugin-tree/src/ObjectTree.tsx': DELEGATING_CALLER, + }); +} + +describe('check-doc-example-shared-reader — the instrument', () => { + /** + * The lit control. Every "no doc comment hand-spells a reader" reading this gate + * produces is worth exactly as much as its ability to say the opposite, and a + * scan that silently walked nothing would report the same zero. This is + * objectui#7638's shape rebuilt: reader present, one call site delegating, the + * `@example` still writing the rung. + */ + it('reports the planted instance (lit control)', () => { + const { findings, counters } = analyze(reseedingTree('schema.objectName', 'lit')); + + expect(counters.documented, 'the walk must have found the documented hook').toBe(1); + expect(counters.callSites, 'the walk must have found the delegating call site').toBe(1); + expect(findings).toHaveLength(1); + expect(findings[0].file).toBe('packages/react/src/hooks/useNavigationOverlay.ts'); + expect(findings[0].symbol).toBe('useNavigationOverlay'); + expect(findings[0].slot).toBe('objectName'); + expect(findings[0].reader).toBe('resolveRecordSourceObjectName'); + expect(findings[0].handSpelled).toBe('schema.objectName'); + }); + + /** + * The control ON the lit control, and the reason the one above is not vacuous. + * + * "A plant reddens it" proves only that the gate reacts to an edit. These three + * plants are edits to the same line in the same doc block, differing only in + * WHAT the example prescribes — and all three must stay green, because a + * literal, a placeholder and a locally-named variable are exactly what an + * example is for. A gate that reddened on these would be a gate over prose + * style, and it would be switched off rather than fixed. + */ + it.each([ + ["'Accounts'", 'a literal'], + ['props.objectName', 'a placeholder the caller supplies'], + ['myResolvedName', 'a locally-named variable'], + ])('stays green when the example prescribes %s (%s)', (spelling) => { + const { findings, counters } = analyze(reseedingTree(spelling, 'nearmiss')); + + // Same population as the lit control — so a green here cannot be a walk that + // found nothing, which is the way this control could itself go vacuous. + expect(counters.documented).toBe(1); + expect(counters.callSites).toBe(1); + expect(findings).toEqual([]); + }); + + /** The other rung of the same reader — the objectui#7627 spelling. */ + it('reports a hand copy of the whole reader body, not only a single rung', () => { + const { findings } = analyze( + reseedingTree("dataConfig?.provider === 'object' ? dataConfig.object : schema.objectName", 'body'), + ); + + expect(findings).toHaveLength(1); + expect(findings[0].handSpelled).toBe("dataConfig.provider === 'object' ? dataConfig.object : schema.objectName"); + }); + + /** + * An example that already delegates is the fixed state, and it is what both + * cards landed. If this reddened, the gate would be unfixable. + */ + it('stays green once the example points at the reader', () => { + const { findings } = analyze(reseedingTree('resolveRecordSourceObjectName(schema, dataConfig)', 'fixed')); + expect(findings).toEqual([]); + }); + + /** + * The trigger is a call site, not a helper. A shared reader published this + * morning with no consumer says nothing about any doc comment, and a gate that + * fired on the reader's mere existence would accuse every example in the tree + * that mentions an object name. + */ + it('says nothing when no call site delegates', () => { + const root = tree('nodelegate', { + 'packages/core/src/utils/record-source.ts': READER, + 'packages/react/src/hooks/useNavigationOverlay.ts': hook('schema.objectName'), + 'packages/plugin-tree/src/ObjectTree.tsx': ` +import { useNavigationOverlay } from '@object-ui/react'; +export function ObjectTree(props: any) { + return useNavigationOverlay({ navigation: props.schema.navigation, objectName: props.schema.objectName }); +} +`, + }); + + const { findings, counters } = analyze(root); + expect(counters.callSites, 'the call site must still have been walked').toBe(1); + expect(findings).toEqual([]); + }); + + /** + * The alias leg, which is where the objectui#7627 instance hides: both sides + * wrap the resolution in a local `const`, and a whole-expression substitution + * sees `schemaKey ?? ''`, finds no binding for it, and reports nothing. + */ + it('sees through a local const on both sides', () => { + const root = tree('alias', { + 'packages/core/src/utils/record-source.ts': READER, + 'packages/react/src/hooks/useSettledSchema.ts': ` +/** + * Settle a schema read. + * + * @example + * \`\`\`tsx + * const schemaKey = dataConfig?.provider === 'object' ? dataConfig.object : schema.objectName; + * const { ready } = useSettledSchema(schemaKey ?? '', dataSource); + * \`\`\` + */ +export function useSettledSchema(key: string, dataSource: unknown) { + return { key, dataSource }; +} +`, + 'packages/plugin-tree/src/ObjectTree.tsx': ` +import { resolveRecordSourceObjectName } from '@object-ui/core'; +import { useSettledSchema } from '@object-ui/react'; +export function ObjectTree(props: any) { + const schemaKey = resolveRecordSourceObjectName(props.schema, props.dataConfig); + return useSettledSchema(schemaKey ?? '', props.dataSource); +} +`, + }); + + const { findings } = analyze(root); + expect(findings).toHaveLength(1); + expect(findings[0].slot).toBe('#0'); + expect(findings[0].handSpelled).toBe("dataConfig.provider === 'object' ? dataConfig.object : schema.objectName"); + }); + + /** + * Tooling files are not documentation anyone copies from, and a test fixture + * legitimately hand-spells whatever it is pinning. Reading them would make the + * gate accuse the pins that hold the readers still. + */ + it('does not read test files as call sites', () => { + const root = tree('tooling', { + 'packages/core/src/utils/record-source.ts': READER, + 'packages/react/src/hooks/useNavigationOverlay.ts': hook('schema.objectName'), + 'packages/plugin-tree/src/__tests__/ObjectTree.test.tsx': DELEGATING_CALLER, + }); + + const { findings, counters } = analyze(root); + expect(counters.callSites).toBe(0); + expect(findings).toEqual([]); + }); +}); + +describe('check-doc-example-shared-reader — the parts', () => { + it('erases optional chaining, which is what makes the copy comparable', () => { + // The reader returns `schema?.objectName`; every copy of it in the tree wrote + // `schema.objectName`. Treating those as different expressions is the one + // normalisation choice that would make the gate blind to its own instance. + expect(canonical('schema?.objectName')).toBe(canonical('schema.objectName')); + expect(canonical('a ??\n b')).toBe('a ?? b'); + }); + + it('reads the alternatives a reader resolves between as its rungs', () => { + const source = `const x = dataConfig?.provider === 'object' ? dataConfig.object : schema?.objectName;`; + const sf = ts.createSourceFile('r.ts', source, ts.ScriptTarget.Latest, true); + const expression = sf.statements[0].declarationList.declarations[0].initializer; + + expect(rungsOf(expression)).toEqual( + expect.arrayContaining([ + "dataConfig.provider === 'object' ? dataConfig.object : schema.objectName", + 'dataConfig.object', + 'schema.objectName', + ]), + ); + }); + + it('matches a rung structurally, never by substring', () => { + // `otherSchema.objectName` CONTAINS the text `schema.objectName`. A substring + // test would fire on it, which is the cheap implementation of this gate and + // the one that would cry wolf. + expect(canonicalContains('schema.objectName', 'schema.objectName')).toBe(true); + expect(canonicalContains("schemaKey ?? ''", 'schema.objectName')).toBe(false); + expect(canonicalContains('otherSchema.objectName', 'schema.objectName')).toBe(false); + }); + + it('does not substitute a binding that is being called', () => { + // `const getDataConfig = ...` is a function, and inlining its body where the + // example CALLS it would fabricate an expression nothing in the tree wrote. + const bindings = new Map([['getDataConfig', 'schema.data ?? null']]); + expect(inline('getDataConfig(schema)', bindings)).toBe('getDataConfig(schema)'); + expect(inline('getDataConfig', bindings)).toBe('(schema.data ?? null)'); + }); + + it('strips the JSDoc line prefix so the fence parses', () => { + const fences = exampleFences(['/**', ' * @example', ' * ```tsx', ' * const a = 1;', ' * ```', ' */'].join('\n')); + expect(fences).toEqual(['const a = 1;\n']); + }); +}); + +describe('check-doc-example-shared-reader — this repository', () => { + const result = analyze(repoRoot); + + /** + * The size guard. A refactor that emptied the walk would satisfy every + * assertion above — they all run on throwaway trees — while this repository's + * run silently checked nothing and reported a pass. + */ + it('walks a population, and compares something in it', () => { + expect(result.counters.files).toBeGreaterThan(200); + expect(result.counters.readers).toBeGreaterThan(5); + expect(result.counters.documented).toBeGreaterThan(5); + expect(result.counters.callSites).toBeGreaterThan(50); + expect( + result.counters.compared, + 'no slot in this repository has a call site delegating to a shared reader, so the ' + + 'gate compared nothing here and its green says nothing', + ).toBeGreaterThan(0); + }); + + it('is green, with the two symbols the cards named among the pairs compared', () => { + expect( + result.findings.map((f) => `${f.file} ${f.symbol}.${f.slot} -> ${f.reader}`), + 'a doc comment in this repository prescribes a spelling a shared reader owns', + ).toEqual([]); + + // Named rather than counted: these two are the reason the gate exists, and a + // refactor that stopped comparing them would leave the green above intact. + const pairs = result.compared.map((c) => `${c.symbol}.${c.slot}`); + expect(pairs).toContain('useNavigationOverlay.objectName'); + expect(pairs).toContain('useSettledSchema.#0'); + }); + + it('keeps the exemption ledger empty', () => { + // An entry here is an admission that a doc comment is live seed text, so it + // carries a card and comes out when that card lands. Empty is the healthy + // state and the gate landed on an empty one. + expect([...KNOWN_HAND_SPELLINGS.keys()]).toEqual([]); + }); + + it('is wired where the sibling parse-based gates run', () => { + const manifest = JSON.parse(fs.readFileSync(path.join(repoRoot, 'package.json'), 'utf8')); + expect(manifest.scripts['check:doc-example-readers']).toBe('node scripts/check-doc-example-shared-reader.mjs'); + + const ci = fs.readFileSync(path.join(repoRoot, '.github/workflows/ci.yml'), 'utf8'); + expect(ci, 'the gate must run in CI, next to the other source-parsing gates').toContain( + 'run: pnpm check:doc-example-readers', + ); + + // The page that inventories the gates is pinned by command (objectui#3653), so + // this would fail there too — asserted here as well because a reader looking + // for this gate looks at this file first. + const page = fs.readFileSync(path.join(repoRoot, 'content/docs/guide/ci-cd-pipeline.md'), 'utf8'); + expect(page).toContain('check:doc-example-readers'); + }); +}); diff --git a/scripts/check-doc-example-shared-reader.mjs b/scripts/check-doc-example-shared-reader.mjs new file mode 100644 index 000000000..557963859 --- /dev/null +++ b/scripts/check-doc-example-shared-reader.mjs @@ -0,0 +1,544 @@ +#!/usr/bin/env node +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * A doc comment must not PRESCRIBE a call-site spelling that a published shared + * reader already owns. + * + * Run: node scripts/check-doc-example-shared-reader.mjs (also `pnpm check:doc-example-readers`) + * node scripts/check-doc-example-shared-reader.mjs --list (every pair this gate compared) + * Exit: 0 = no `@example` hand-spells a reader its own call sites delegate to, + * 1 = at least one does, or the scan collapsed. + * + * ## The gap this closes (objectui#7652) + * + * A JSDoc `@example` on an exported hook or helper is not decoration: call sites + * copy it. When the ruling the example encoded moves, the prose stays — and every + * later copy is seeded from the prose, not from the code. Fixing the call sites + * without fixing the prose re-seeds them. + * + * Measured cost before this gate existed: two cards and three copied call sites. + * + * objectui#7627 `useSettledSchema`'s `@example` spelled the record-source + * ladder inline (`dataConfig?.provider === 'object' ? …`). Six + * view plugins carried their own copy of it, drifted three ways. + * objectui#7638 `useNavigationOverlay`'s `@example` passed the bare + * `schema.objectName`. Three components copied that line while + * resolving their record source the other way in the same file. + * + * Both were closed by pointing the prose at `resolveRecordSourceObjectName` from + * `@object-ui/core` — the ONE reader objectui#7627 published for that ladder. + * Nothing in CI could see either one. + * + * ## What this gate answers, and what it deliberately does NOT + * + * It answers exactly one question: + * + * **when the real in-repo call sites of a documented symbol obtain an argument + * by CALLING a shared reader, does the symbol's own `@example` obtain it the + * same way — or does it still hand-spell what that reader owns?** + * + * It does NOT answer, on purpose, and each of these is a real limit rather than + * an oversight: + * + * 1. **Whether the spelling a doc comment prescribes is CORRECT.** That is a + * ruling, and a ruling is not in the tree. Both instances above were, on the + * day they were filed, in a state where the prose and every copy of it + * AGREED — nothing in the repository disagreed with the doc comment, so no + * gate reading only the tree could have known the prose was wrong. What this + * gate catches is the state immediately AFTER: the call sites move to the + * shared reader and the prose does not. That transition is the moment the + * prose becomes a seed, and it is the one the two cards above both name. + * 2. **Prose outside a fenced `@example`.** A `@param` line that prescribes a + * spelling in running text is the same defect (objectui#7627's did), but the + * fence is where a copier's cursor goes and it is the part that parses. + * Extending to `@param` needs a way to tell a prescription from an aside, + * and guessing that boundary is what produces a gate people learn to ignore. + * 3. **Documentation under `content/docs/**`.** That surface belongs to + * `check-doc-snippet-types.mjs` (does the snippet still compile) and + * `check-doc-component-types.mjs` (does the `type` it names exist). This + * gate reads doc comments in `packages//src` only. + * 4. **A reader nobody calls yet.** The trigger is a real call site, not the + * existence of a helper. A shared reader published this morning with no + * consumer says nothing about any doc comment. + * 5. **Whether the reader the call sites use is the RIGHT reader.** It compares + * the doc against the call sites; it does not grade either against a spec. + * + * ## Why this is not `check-spec-symbol-derivation.mjs` (objectui#7617 was + * mis-cited for this class, twice) + * + * That gate's rule 4 judges citations of `@objectstack/spec` at MEMBER + * granularity — a docblock naming `NavigationConfigSchema.zzzNotARealMember` + * fails it. Measured on `useNavigationOverlay.ts`, which carried objectui#7638's + * instance: baseline exit 0 with zero mentions of the file; plant a dangling spec + * member in that same doc block and it goes to exit 1 naming the file. So it DOES + * read the file — it simply has nothing to say about prose that prescribes a + * LOCAL spelling and cites no spec symbol. The zero was a reading, not a dead + * instrument, and objectui#7638's card and the dispatch that followed it both + * recorded the coverage anyway. This file is the gate that was missing. + * + * ## The narrowing, stated as a rule + * + * A finding needs all four of these to hold at once: + * + * a. an exported symbol `S` in `packages//src` whose JSDoc carries a fenced + * `ts`/`tsx` `@example` that CALLS `S`; + * b. at least one real in-repo call site of `S`, outside `S`'s own file, whose + * expression for the same argument slot CALLS an exported single-`return` + * reader `R`; + * c. the `@example`'s expression for that slot does NOT call `R`; + * d. that expression is structurally what `R` itself resolves — equal to `R`'s + * whole return expression, or to one of its RUNGS (the branches of its + * conditional, the operands of its `??`/`||` chain), written with `R`'s own + * parameter spellings. + * + * (d) is what keeps this off ordinary examples. A doc comment is allowed to pass + * a literal, a placeholder, or a locally-named variable where a call site passes + * something else — that is what an example is for. It is not allowed to spell out + * the body of the reader its callers delegate to, because that spelling is the + * thing that gets copied. + * + * Local `const` bindings are inlined one level on both sides before comparison, + * because both sides of every real instance were written that way: + * `const schemaKey = resolveRecordSourceObjectName(schema, dataConfig)` at the + * call site, and the same shape in the example. + * + * ## Rollout + * + * `KNOWN_HAND_SPELLINGS` below is empty and is an allowlist that only shrinks: an + * entry is a live defect with a card, never a waiver. It exists so that a first + * run finding real instances cannot force the prose fixes into the same PR as the + * gate — the same shape `scripts/__tests__/network-escape-ledger.test.ts` uses. + */ + +import { readFileSync, readdirSync, statSync } from 'node:fs'; +import { dirname, join, relative, resolve, sep } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import ts from 'typescript'; + +import { TOOLING_FILE, listSourceFiles } from './check-phantom-dependencies.mjs'; +import { isEntrypoint } from './invoked-as.mjs'; + +const scriptDir = dirname(fileURLToPath(import.meta.url)); + +/** + * Doc comments known to hand-spell a reader their call sites delegate to, each + * with the card that decides the prose. + * + * Empty is the healthy state. An entry is an admission that a doc comment is + * live seed text, not a decision that it is fine — so it carries a card, and it + * comes out when that card lands. The key is `file::symbol::slot`. + */ +export const KNOWN_HAND_SPELLINGS = new Map([]); + +/** Packages are the population: this gate reads doc comments that ship. */ +export function populationFiles(root) { + const packagesDir = resolve(root, 'packages'); + const files = []; + let entries; + try { + entries = readdirSync(packagesDir, { withFileTypes: true }); + } catch { + return files; + } + for (const entry of entries.sort((a, b) => a.name.localeCompare(b.name))) { + if (!entry.isDirectory() || entry.name.startsWith('.')) continue; + const src = join(packagesDir, entry.name, 'src'); + try { + if (!statSync(src).isDirectory()) continue; + } catch { + continue; + } + for (const file of listSourceFiles(src)) { + const rel = relative(root, file).split(sep).join('/'); + if (TOOLING_FILE.test(rel)) continue; + if (rel.endsWith('.d.ts')) continue; + files.push(file); + } + } + return files; +} + +export function parseSource(text, fileName) { + return ts.createSourceFile(fileName, text, ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX); +} + +/** + * The canonical form two expressions are compared in. + * + * Optional chaining is erased because it is a null-safety choice, not a + * different read: the reader below returns `schema?.objectName` while every + * copy of it in the tree wrote `schema.objectName`, and treating those as + * different expressions would make the gate blind to the exact instance it + * exists for. Parens and non-null assertions go for the same reason. + */ +export function canonical(text) { + return String(text) + .replace(/\/\*[\s\S]*?\*\//g, ' ') + .replace(/\?\./g, '.') + .replace(/!\s*\./g, '.') + .replace(/\s+/g, ' ') + .trim(); +} + +/** Does this expression text call `name`? Asked of text, so an inlined alias counts. */ +export function callsFunction(text, name) { + return new RegExp(`\\b${name}\\s*\\(`).test(String(text)); +} + +/** + * Every exported function whose body is exactly one `return`, with the rungs it + * resolves between. + * + * A single-`return` export is this repository's shape for "the one spelling of a + * resolution" — `resolveRecordSourceObjectName` is one line. A helper with + * statements in it is doing something an example could not be hand-spelling in a + * single argument, so it is not a candidate here. + */ +export function readersIn(sourceFile, relPath) { + const readers = []; + const visit = (node) => { + if (ts.isFunctionDeclaration(node) && node.name && node.body) { + const exported = (ts.getModifiers(node) ?? []).some((m) => m.kind === ts.SyntaxKind.ExportKeyword); + const statements = node.body.statements; + if (exported && statements.length === 1 && ts.isReturnStatement(statements[0]) && statements[0].expression) { + const expression = statements[0].expression; + readers.push({ + name: node.name.text, + file: relPath, + params: node.parameters.map((p) => p.name.getText()), + body: canonical(expression.getText()), + rungs: rungsOf(expression), + }); + } + } + ts.forEachChild(node, visit); + }; + ts.forEachChild(sourceFile, visit); + return readers; +} + +/** + * The alternatives a reader resolves BETWEEN — the thing a hand copy writes out + * one of. For `a ? b : c` those are `b` and `c`; for `a ?? b` they are `a` and + * `b`; nested chains contribute each leaf. The whole expression is always a rung + * of itself, which is how a copy of the entire body is caught. + */ +export function rungsOf(expression) { + const out = new Set([canonical(expression.getText())]); + const walk = (node) => { + if (ts.isParenthesizedExpression(node)) return walk(node.expression); + if (ts.isConditionalExpression(node)) { + walk(node.whenTrue); + walk(node.whenFalse); + return; + } + if ( + ts.isBinaryExpression(node) && + (node.operatorToken.kind === ts.SyntaxKind.QuestionQuestionToken || + node.operatorToken.kind === ts.SyntaxKind.BarBarToken) + ) { + walk(node.left); + walk(node.right); + return; + } + out.add(canonical(node.getText())); + }; + walk(expression); + return [...out]; +} + +/** `@example` fences, with the JSDoc line prefix removed so the code parses. */ +export function exampleFences(jsdocText) { + const body = jsdocText + .replace(/^\s*\/\*\*/, '') + .replace(/\*\/\s*$/, '') + .split('\n') + .map((line) => line.replace(/^\s*\* ?/, '')) + .join('\n'); + if (!body.includes('@example')) return []; + return [...body.matchAll(/```(?:tsx?|jsx?|typescript)\n([\s\S]*?)```/g)].map((m) => m[1]); +} + +/** Argument slots of one call: named for an options object, `#i` for positional. */ +export function slotsOf(call) { + const slots = new Map(); + call.arguments.forEach((argument, index) => { + if (ts.isObjectLiteralExpression(argument)) { + for (const property of argument.properties) { + if (ts.isPropertyAssignment(property) && (ts.isIdentifier(property.name) || ts.isStringLiteral(property.name))) { + slots.set(property.name.text, property.initializer.getText()); + } else if (ts.isShorthandPropertyAssignment(property)) { + slots.set(property.name.text, property.name.text); + } + } + return; + } + slots.set(`#${index}`, argument.getText()); + }); + return slots; +} + +/** `const NAME = ;` bindings visible from `node`, innermost first. */ +export function bindingsFor(node) { + const bindings = new Map(); + const scopes = []; + for (let current = node; current; current = current.parent) { + if (ts.isBlock(current) || ts.isSourceFile(current) || ts.isModuleBlock(current)) scopes.push(current); + } + for (const scope of scopes.reverse()) { + for (const statement of scope.statements) { + if (!ts.isVariableStatement(statement)) continue; + for (const declaration of statement.declarationList.declarations) { + if (ts.isIdentifier(declaration.name) && declaration.initializer) { + bindings.set(declaration.name.text, declaration.initializer.getText()); + } + } + } + } + return bindings; +} + +/** + * Substitute local `const` aliases INTO the expression, two passes deep. + * + * Token-level rather than whole-expression, because both sides of every real + * instance wrap the alias in something: the call site writes + * `useSettledSchema(schemaKey ?? '', ...)` while `schemaKey` is where the + * resolution actually lives. A whole-expression substitution sees `schemaKey ?? + * ''`, finds no binding for it, and the resolution stays invisible — measured: + * objectui#7627's own instance is silent without this. + * + * A name is substituted at most once per run, so a self-referential binding + * (`const x = x ?? y`) terminates instead of expanding forever. + */ +export function inline(expression, bindings) { + let text = String(expression); + const used = new Set(); + for (let pass = 0; pass < 2; pass += 1) { + let changed = false; + for (const [name, value] of bindings) { + if (used.has(name)) continue; + const token = new RegExp(`\\b${name}\\b`); + if (!token.test(text)) continue; + // Not a substitution when the name is being CALLED — `foo(x)` names a + // function, and replacing it with the function's own initializer would + // fabricate an expression nothing in the tree wrote. + if (new RegExp(`\\b${name}\\s*\\(`).test(text)) continue; + used.add(name); + text = text.replace(new RegExp(`\\b${name}\\b`, 'g'), `(${value})`); + changed = true; + } + if (!changed) break; + } + return text; +} + +export function analyze(root) { + const files = populationFiles(root); + const counters = { files: files.length, readers: 0, documented: 0, compared: 0, callSites: 0 }; + const parsed = new Map(); + const readers = new Map(); + + for (const file of files) { + let text; + try { + text = readFileSync(file, 'utf8'); + } catch { + continue; + } + const rel = relative(root, file).split(sep).join('/'); + const sourceFile = parseSource(text, rel); + parsed.set(rel, sourceFile); + for (const reader of readersIn(sourceFile, rel)) { + if (!readers.has(reader.name)) readers.set(reader.name, reader); + } + } + counters.readers = readers.size; + + // Exported symbols whose own `@example` calls them. + const documented = new Map(); + for (const [rel, sourceFile] of parsed) { + const visit = (node) => { + let name = null; + if (ts.isFunctionDeclaration(node) && node.name) name = node.name.text; + else if (ts.isVariableStatement(node)) { + const declaration = node.declarationList.declarations[0]; + if (declaration && ts.isIdentifier(declaration.name)) name = declaration.name.text; + } + if (name && !documented.has(name)) { + const exported = (ts.getModifiers(node) ?? []).some((m) => m.kind === ts.SyntaxKind.ExportKeyword); + if (exported) { + for (const doc of ts.getJSDocCommentsAndTags(node).filter(ts.isJSDoc)) { + for (const fence of exampleFences(doc.getText())) { + const fenceFile = parseSource(fence, 'example.tsx'); + let call = null; + const find = (n) => { + if (!call && ts.isCallExpression(n) && ts.isIdentifier(n.expression) && n.expression.text === name) { + call = n; + } + ts.forEachChild(n, find); + }; + ts.forEachChild(fenceFile, find); + if (!call) continue; + const bindings = bindingsFor(call); + const slots = new Map(); + for (const [slot, expression] of slotsOf(call)) slots.set(slot, inline(expression, bindings)); + documented.set(name, { file: rel, slots }); + break; + } + if (documented.has(name)) break; + } + } + } + ts.forEachChild(node, visit); + }; + ts.forEachChild(sourceFile, visit); + } + counters.documented = documented.size; + + // Real call sites of those symbols, outside the file that declares them. + const callSites = new Map(); + for (const [rel, sourceFile] of parsed) { + const visit = (node) => { + if (ts.isCallExpression(node) && ts.isIdentifier(node.expression)) { + const name = node.expression.text; + const target = documented.get(name); + if (target && target.file !== rel) { + const bindings = bindingsFor(node); + const slots = new Map(); + for (const [slot, expression] of slotsOf(node)) slots.set(slot, inline(expression, bindings)); + if (!callSites.has(name)) callSites.set(name, []); + callSites.get(name).push({ file: rel, slots }); + counters.callSites += 1; + } + } + ts.forEachChild(node, visit); + }; + ts.forEachChild(sourceFile, visit); + } + + const findings = []; + const compared = []; + for (const [name, doc] of documented) { + const sites = callSites.get(name) ?? []; + if (!sites.length) continue; + for (const [slot, docExpression] of doc.slots) { + const delegating = new Map(); + for (const site of sites) { + const siteExpression = site.slots.get(slot); + if (siteExpression === undefined) continue; + for (const reader of readers.values()) { + if (reader.name === name) continue; + if (callsFunction(siteExpression, reader.name)) { + if (!delegating.has(reader.name)) delegating.set(reader.name, []); + delegating.get(reader.name).push(site.file); + } + } + } + if (!delegating.size) continue; + counters.compared += 1; + for (const [readerName, users] of delegating) { + const reader = readers.get(readerName); + compared.push({ symbol: name, slot, reader: readerName, users: users.length }); + if (callsFunction(docExpression, readerName)) continue; + const handSpelled = reader.rungs.find((rung) => canonicalContains(docExpression, rung)); + if (!handSpelled) continue; + const key = `${doc.file}::${name}::${slot}`; + if (KNOWN_HAND_SPELLINGS.has(key)) continue; + findings.push({ + key, + file: doc.file, + symbol: name, + slot, + docExpression: canonical(docExpression), + reader: readerName, + readerFile: reader.file, + handSpelled, + users, + }); + } + } + } + return { findings, counters, compared }; +} + +/** + * Is `rung` the whole of this expression, or one of the alternatives it resolves + * between? Asked structurally rather than by substring, so a reader rung of + * `schema.objectName` does not fire on an unrelated `otherSchema.objectName`. + */ +export function canonicalContains(expressionText, rung) { + const parsedExpression = parseSource(`const __probe = (${expressionText});`, 'probe.tsx'); + const statement = parsedExpression.statements[0]; + if (!ts.isVariableStatement(statement)) return false; + const initializer = statement.declarationList.declarations[0]?.initializer; + if (!initializer) return false; + let hit = false; + const walk = (node) => { + if (hit) return; + if (canonical(node.getText()) === rung) { + hit = true; + return; + } + ts.forEachChild(node, walk); + }; + walk(initializer); + return hit; +} + +const invokedDirectly = isEntrypoint(import.meta.url); + +if (invokedDirectly) { + const argOf = (name) => { + const index = process.argv.indexOf(name); + return index > -1 ? process.argv[index + 1] : null; + }; + const root = resolve(argOf('--root') ?? resolve(scriptDir, '..')); + const { findings, counters, compared } = analyze(root); + + if (process.argv.includes('--list')) { + for (const entry of compared) { + console.log(`${entry.symbol}.${entry.slot} <- ${entry.reader} (${entry.users} call site(s))`); + } + } + + // A refactor that quietly emptied the walk would satisfy every assertion in the + // pin while checking nothing — the same size guard the sibling gates open with. + if (counters.files < 200 || counters.readers < 5 || counters.documented < 5) { + console.error( + `The scan collapsed: ${counters.files} source file(s), ${counters.readers} single-return export(s), ` + + `${counters.documented} documented symbol(s) whose @example calls them. An empty comparison would ` + + 'pass while asserting nothing.', + ); + process.exit(1); + } + + if (!findings.length) { + console.log( + `OK ${counters.documented} documented symbol(s), ${counters.callSites} call site(s), ` + + `${counters.compared} slot(s) where a call site delegates to a shared reader — no @example hand-spells one.`, + ); + process.exit(0); + } + + console.error( + `x ${findings.length} doc comment(s) prescribe a spelling a shared reader already owns:\n`, + ); + for (const finding of findings) { + console.error( + ` ${finding.file} ${finding.symbol}({ ${finding.slot}: ${finding.docExpression} })\n` + + ` ${finding.users.length} call site(s) pass this slot through ${finding.reader}() ` + + `(${finding.readerFile}); the example still writes \`${finding.handSpelled}\`, which is what that ` + + `reader resolves.\n` + + ` Call sites: ${finding.users.join(', ')}`, + ); + } + console.error( + '\nThe example is what the next call site is copied from, so prose that outlives the ruling re-seeds\n' + + 'the copies (objectui#7627, objectui#7638). Point the example at the reader, or add the key to\n' + + 'KNOWN_HAND_SPELLINGS in scripts/check-doc-example-shared-reader.mjs with the card that decides it.', + ); + process.exit(1); +} From 4cb43094c17723c70b0f07558df8fb1a1c430762 Mon Sep 17 00:00:00 2001 From: os-sam Date: Sat, 5 Sep 2026 19:24:29 +0000 Subject: [PATCH 2/3] fix(tooling): read every doc block, not only the one attached to the symbol it calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first run of the widened scan found a live instance the narrower rule was blind to: `packages/components/src/custom/navigation-overlay.tsx` carries a FILE-HEADER block documenting `NavigationOverlay` whose `@example` calls `useNavigationOverlay` and still teaches `objectName: schema.objectName` — the spelling objectui#7638 was filed about and PR #7648 removed from the hook's own doc block one package over. Different file, different package, a block attached to no declaration: nothing in that fix pointed at it. Comment spans come from `js-comment-mask.mjs` rather than a regex, since this is a gate whose entire subject is comment text. Comparison is restricted to symbols this repository exports, because once every block is read an example calling `useMemo`/`useEffect`/`fetch` compares against every reader in the tree. The instance is reported, not fixed: objectui#7787 decides the prose, and it is carried as `KNOWN_HAND_SPELLINGS`' only row so the gate lands provably green rather than bundled with a change to what it judges. A row whose defect is gone fails the gate. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3 --- .../check-doc-example-shared-reader.test.ts | 117 ++++++++- scripts/check-doc-example-shared-reader.mjs | 228 +++++++++++++----- 2 files changed, 280 insertions(+), 65 deletions(-) diff --git a/scripts/__tests__/check-doc-example-shared-reader.test.ts b/scripts/__tests__/check-doc-example-shared-reader.test.ts index 9d47118cd..aca7428e3 100644 --- a/scripts/__tests__/check-doc-example-shared-reader.test.ts +++ b/scripts/__tests__/check-doc-example-shared-reader.test.ts @@ -10,6 +10,7 @@ import { analyze, canonical, canonicalContains, + docBlocks, exampleFences, inline, rungsOf, @@ -259,6 +260,73 @@ export function ObjectTree(props: any) { * legitimately hand-spells whatever it is pinning. Reading them would make the * gate accuse the pins that hold the readers still. */ + /** + * The widening that the first run made non-optional. A JSDoc block attached to + * NO declaration — a file header — teaching a call to a symbol declared in a + * different package is the shape `navigation-overlay.tsx` carries, and a gate + * that only read JSDoc attached to the symbol it documents would report a clean + * tree over it. + */ + it('reads a file-header block that documents a different symbol', () => { + const root = tree('header', { + 'packages/core/src/utils/record-source.ts': READER, + 'packages/react/src/hooks/useNavigationOverlay.ts': hook('resolveRecordSourceObjectName(schema, dataConfig)'), + 'packages/components/src/custom/navigation-overlay.tsx': ` +/** + * NavigationOverlay + * + * Works in conjunction with useNavigationOverlay from @object-ui/react. + * + * @example + * \`\`\`tsx + * const nav = useNavigationOverlay({ navigation: schema.navigation, objectName: schema.objectName }); + * \`\`\` + */ +import React from 'react'; +export function NavigationOverlay() { + return React.createElement('div'); +} +`, + 'packages/plugin-tree/src/ObjectTree.tsx': DELEGATING_CALLER, + }); + + const { raw } = analyze(root); + expect(raw.map((f) => f.file)).toEqual(['packages/components/src/custom/navigation-overlay.tsx']); + }); + + /** + * The other half of that widening. Once every block comment is read, an example + * calling `useMemo`, `useEffect` or `fetch` becomes a comparison against every + * reader in the tree — hundreds of meaningless pairs and a latent false + * positive. Only what this repository exports is compared. + */ + it('does not compare a call to something this repository does not export', () => { + const root = tree('foreign', { + 'packages/core/src/utils/record-source.ts': READER, + 'packages/react/src/hooks/useThing.ts': ` +/** + * @example + * \`\`\`tsx + * const value = useMemo(() => schema.objectName, [schema]); + * \`\`\` + */ +export function useThing() { + return null; +} +`, + 'packages/plugin-tree/src/ObjectTree.tsx': ` +import { resolveRecordSourceObjectName } from '@object-ui/core'; +export function ObjectTree(props: any) { + return useMemo(() => resolveRecordSourceObjectName(props.schema, props.dataConfig), [props]); +} +`, + }); + + const { raw, counters } = analyze(root); + expect(counters.documented, '`useMemo` is not first-party, so no example call is comparable').toBe(0); + expect(raw).toEqual([]); + }); + it('does not read test files as call sites', () => { const root = tree('tooling', { 'packages/core/src/utils/record-source.ts': READER, @@ -312,6 +380,14 @@ describe('check-doc-example-shared-reader — the parts', () => { expect(inline('getDataConfig', bindings)).toBe('(schema.data ?? null)'); }); + it('finds block comments through the shared scanner, not a regex', () => { + // A block-comment OPENER inside a string literal is what breaks the naive + // regex: it opens a phantom comment that runs to the next real terminator, + // swallowing the real doc block below it. + const source = ["const glob = '/*.ts';", '/** @example real */', 'export const x = 1;'].join('\n'); + expect(docBlocks(source)).toEqual(['/** @example real */']); + }); + it('strips the JSDoc line prefix so the fence parses', () => { const fences = exampleFences(['/**', ' * @example', ' * ```tsx', ' * const a = 1;', ' * ```', ' */'].join('\n')); expect(fences).toEqual(['const a = 1;\n']); @@ -351,11 +427,42 @@ describe('check-doc-example-shared-reader — this repository', () => { expect(pairs).toContain('useSettledSchema.#0'); }); - it('keeps the exemption ledger empty', () => { - // An entry here is an admission that a doc comment is live seed text, so it - // carries a card and comes out when that card lands. Empty is the healthy - // state and the gate landed on an empty one. - expect([...KNOWN_HAND_SPELLINGS.keys()]).toEqual([]); + /** + * The ledger is an allowlist that only shrinks. Two directions are pinned, + * because a waiver can go wrong both ways: a row whose defect is gone reads as + * a live waiver for nothing, and a row with no reason is indistinguishable + * from switching the gate off for that file. + */ + it('keeps every exemption honest — no stale row, and a reason on each', () => { + expect( + result.stale, + 'a KNOWN_HAND_SPELLINGS row names a doc comment this gate no longer finds — the defect ' + + 'it waives is fixed, so the row is a live waiver for nothing. Delete it.', + ).toEqual([]); + + for (const [key, reason] of KNOWN_HAND_SPELLINGS) { + expect(reason.length, `KNOWN_HAND_SPELLINGS[${key}] must carry a real justification`).toBeGreaterThan(40); + expect(reason, `KNOWN_HAND_SPELLINGS[${key}] must name the card that decides the prose`).toMatch( + /objectui#\d+/, + ); + } + }); + + /** + * The gate's first run over this repository found one instance, and it is a + * real one rather than a false positive: `navigation-overlay.tsx`'s file-header + * `@example` still teaches objectui#7638's spelling, one file over from the doc + * block PR #7648 fixed. objectui#7652 fenced the prose fixes out of this PR, so + * it is carried as the ledger's only row and named here — a count would let it + * be swapped for a different waiver without anyone noticing. + */ + it('carries objectui#7787 as its one waived instance, and nothing else', () => { + expect([...KNOWN_HAND_SPELLINGS.keys()]).toEqual([ + 'packages/components/src/custom/navigation-overlay.tsx::useNavigationOverlay::objectName', + ]); + expect(result.raw.map((f) => f.key)).toContain( + 'packages/components/src/custom/navigation-overlay.tsx::useNavigationOverlay::objectName', + ); }); it('is wired where the sibling parse-based gates run', () => { diff --git a/scripts/check-doc-example-shared-reader.mjs b/scripts/check-doc-example-shared-reader.mjs index 557963859..49c137963 100644 --- a/scripts/check-doc-example-shared-reader.mjs +++ b/scripts/check-doc-example-shared-reader.mjs @@ -54,6 +54,8 @@ * fence is where a copier's cursor goes and it is the part that parses. * Extending to `@param` needs a way to tell a prescription from an aside, * and guessing that boundary is what produces a gate people learn to ignore. + * Nor does it read `//` line comments, or any comment that does not open + * `/**`. * 3. **Documentation under `content/docs/**`.** That surface belongs to * `check-doc-snippet-types.mjs` (does the snippet still compile) and * `check-doc-component-types.mjs` (does the `type` it names exist). This @@ -81,9 +83,14 @@ * * A finding needs all four of these to hold at once: * - * a. an exported symbol `S` in `packages//src` whose JSDoc carries a fenced - * `ts`/`tsx` `@example` that CALLS `S`; - * b. at least one real in-repo call site of `S`, outside `S`'s own file, whose + * a. a JSDoc block anywhere in `packages//src` whose fenced `ts`/`tsx` + * `@example` CALLS a symbol `S` this repository exports. Anywhere, and not + * only the block attached to `S`: the live instance this gate found on its + * first run is a FILE-HEADER block documenting `NavigationOverlay` in + * `packages/components` whose example calls `useNavigationOverlay` from + * `packages/react` (objectui#7787). First-party, because an example calls + * `useMemo` and `fetch` too and comparing those means nothing here; + * b. at least one real in-repo call site of `S`, in another file, whose * expression for the same argument slot CALLS an exported single-`return` * reader `R`; * c. the `@example`'s expression for that slot does NOT call `R`; @@ -105,10 +112,15 @@ * * ## Rollout * - * `KNOWN_HAND_SPELLINGS` below is empty and is an allowlist that only shrinks: an - * entry is a live defect with a card, never a waiver. It exists so that a first - * run finding real instances cannot force the prose fixes into the same PR as the - * gate — the same shape `scripts/__tests__/network-escape-ledger.test.ts` uses. + * `KNOWN_HAND_SPELLINGS` below is an allowlist that only shrinks: an entry is a + * live defect with a card, never a waiver. It exists so that a first run finding + * real instances cannot force the prose fixes into the same PR as the gate — the + * same shape `scripts/__tests__/network-escape-ledger.test.ts` uses. + * + * It landed carrying exactly one row, which is the gate's own first finding: + * objectui#7787, `navigation-overlay.tsx`'s file-header example, the copy of + * objectui#7638's spelling that PR #7648's fix did not reach. A row whose defect + * is gone fails this gate rather than sitting there as a waiver for nothing. */ import { readFileSync, readdirSync, statSync } from 'node:fs'; @@ -119,6 +131,7 @@ import ts from 'typescript'; import { TOOLING_FILE, listSourceFiles } from './check-phantom-dependencies.mjs'; import { isEntrypoint } from './invoked-as.mjs'; +import { scanSource } from './js-comment-mask.mjs'; const scriptDir = dirname(fileURLToPath(import.meta.url)); @@ -130,7 +143,32 @@ const scriptDir = dirname(fileURLToPath(import.meta.url)); * live seed text, not a decision that it is fine — so it carries a card, and it * comes out when that card lands. The key is `file::symbol::slot`. */ -export const KNOWN_HAND_SPELLINGS = new Map([]); +export const KNOWN_HAND_SPELLINGS = new Map([ + [ + 'packages/components/src/custom/navigation-overlay.tsx::useNavigationOverlay::objectName', + "objectui#7787. `navigation-overlay.tsx`'s FILE-HEADER block documents " + + '`NavigationOverlay` while its `@example` calls `useNavigationOverlay`, and it still ' + + "teaches `objectName: schema.objectName` — the exact spelling objectui#7638 was filed " + + "about and PR #7648 removed from the hook's own doc block. That fix did not reach this " + + 'file: different file, different package, a block documenting a different symbol. It is ' + + 'the first thing this gate found and the reason the gate is worth having, and it is NOT ' + + "fixed here on purpose — objectui#7652 fenced the prose fixes out of the gate's own PR, " + + 'so that the gate lands provably green rather than bundled with a change to what it ' + + 'judges. Delete this row in the same change that fixes the example.', + ], +]); + +/** + * A ledger row naming a doc comment this gate no longer finds is worse than no + * ledger: it reads as a live waiver for a defect that is gone, and the next + * person to fix a real one has no way to tell the two apart. So every row must + * still correspond to something the scan reports, and `analyze` returns the + * stale ones for the pin to fail on. + */ +export function staleExemptions(rawFindings) { + const live = new Set(rawFindings.map((finding) => finding.key)); + return [...KNOWN_HAND_SPELLINGS.keys()].filter((key) => !live.has(key)); +} /** Packages are the population: this gate reads doc comments that ship. */ export function populationFiles(root) { @@ -187,6 +225,28 @@ export function callsFunction(text, name) { return new RegExp(`\\b${name}\\s*\\(`).test(String(text)); } +/** Names this repository exports — the first-party surface a doc comment teaches. */ +export function exportedNames(sourceFile) { + const names = new Set(); + const visit = (node) => { + const exported = (ts.getModifiers(node) ?? []).some((m) => m.kind === ts.SyntaxKind.ExportKeyword); + if (exported) { + if ((ts.isFunctionDeclaration(node) || ts.isClassDeclaration(node)) && node.name) names.add(node.name.text); + else if (ts.isVariableStatement(node)) { + for (const declaration of node.declarationList.declarations) { + if (ts.isIdentifier(declaration.name)) names.add(declaration.name.text); + } + } + } + if (ts.isExportDeclaration(node) && node.exportClause && ts.isNamedExports(node.exportClause)) { + for (const element of node.exportClause.elements) names.add(element.name.text); + } + ts.forEachChild(node, visit); + }; + ts.forEachChild(sourceFile, visit); + return names; +} + /** * Every exported function whose body is exactly one `return`, with the rungs it * resolves between. @@ -249,6 +309,31 @@ export function rungsOf(expression) { return [...out]; } +/** + * Every `/**`-opening block comment in a source, read through the shared scanner + * rather than a regex. + * + * `js-comment-mask.mjs` exists because the naive regex opens a PHANTOM comment on + * a block-comment opener inside a string literal and then deletes every line to + * the next terminator — reporting clean over text it never looked at. A gate + * whose entire subject is comment text is the last place to re-derive that. + */ +export function docBlocks(source) { + const { comment } = scanSource(source); + const blocks = []; + let start = -1; + for (let index = 0; index < source.length; index += 1) { + if (comment[index] && start === -1) start = index; + else if (!comment[index] && start !== -1) { + const span = source.slice(start, index); + if (span.startsWith('/**')) blocks.push(span); + start = -1; + } + } + if (start !== -1 && source.slice(start).startsWith('/**')) blocks.push(source.slice(start)); + return blocks; +} + /** `@example` fences, with the JSDoc line prefix removed so the code parses. */ export function exampleFences(jsdocText) { const body = jsdocText @@ -337,9 +422,10 @@ export function inline(expression, bindings) { export function analyze(root) { const files = populationFiles(root); - const counters = { files: files.length, readers: 0, documented: 0, compared: 0, callSites: 0 }; + const counters = { files: files.length, readers: 0, exported: 0, documented: 0, compared: 0, callSites: 0 }; const parsed = new Map(); const readers = new Map(); + const exported = new Set(); for (const file of files) { let text; @@ -354,65 +440,75 @@ export function analyze(root) { for (const reader of readersIn(sourceFile, rel)) { if (!readers.has(reader.name)) readers.set(reader.name, reader); } + for (const name of exportedNames(sourceFile)) exported.add(name); } counters.readers = readers.size; + counters.exported = exported.size; - // Exported symbols whose own `@example` calls them. + // Every JSDoc block in the population, and the calls its `@example` fences make. + // + // Comment spans come from `js-comment-mask.mjs` rather than a regex: this is a + // gate whose whole subject is comment text, and the naive + // `/\/\*[\s\S]*?\*\//` family opens a phantom comment on any block-comment + // opener inside a string literal, which this tree really writes. + // + // Deliberately NOT restricted to a JSDoc attached to the symbol it calls. The + // live instance that made this widening non-optional is + // `packages/components/src/custom/navigation-overlay.tsx`: a FILE-HEADER block + // documenting `NavigationOverlay` whose `@example` calls + // `useNavigationOverlay` — a different symbol, in a different package, from a + // comment attached to no declaration at all. It carries objectui#7638's exact + // spelling and survived that card's fix untouched, which is the class this gate + // exists for happening one file over. const documented = new Map(); for (const [rel, sourceFile] of parsed) { - const visit = (node) => { - let name = null; - if (ts.isFunctionDeclaration(node) && node.name) name = node.name.text; - else if (ts.isVariableStatement(node)) { - const declaration = node.declarationList.declarations[0]; - if (declaration && ts.isIdentifier(declaration.name)) name = declaration.name.text; - } - if (name && !documented.has(name)) { - const exported = (ts.getModifiers(node) ?? []).some((m) => m.kind === ts.SyntaxKind.ExportKeyword); - if (exported) { - for (const doc of ts.getJSDocCommentsAndTags(node).filter(ts.isJSDoc)) { - for (const fence of exampleFences(doc.getText())) { - const fenceFile = parseSource(fence, 'example.tsx'); - let call = null; - const find = (n) => { - if (!call && ts.isCallExpression(n) && ts.isIdentifier(n.expression) && n.expression.text === name) { - call = n; - } - ts.forEachChild(n, find); - }; - ts.forEachChild(fenceFile, find); - if (!call) continue; - const bindings = bindingsFor(call); + const text = sourceFile.getFullText(); + for (const block of docBlocks(text)) { + for (const fence of exampleFences(block)) { + const fenceFile = parseSource(fence, 'example.tsx'); + const find = (node) => { + if (ts.isCallExpression(node) && ts.isIdentifier(node.expression)) { + const name = node.expression.text; + // First-party only. An example calls `useMemo`, `useEffect` and + // `fetch` too, and comparing those against every reader in the tree + // is a large surface of meaningless work — and a latent false + // positive — for a gate whose subject is THIS repository's own + // documented surface. Measured before narrowing: the population went + // from 8 comparable symbols to hundreds, all of them React or global. + if (!exported.has(name)) { + ts.forEachChild(node, find); + return; + } + const key = `${rel}::${name}`; + if (!documented.has(key)) { + const bindings = bindingsFor(node); const slots = new Map(); - for (const [slot, expression] of slotsOf(call)) slots.set(slot, inline(expression, bindings)); - documented.set(name, { file: rel, slots }); - break; + for (const [slot, expression] of slotsOf(node)) slots.set(slot, inline(expression, bindings)); + documented.set(key, { file: rel, symbol: name, slots }); } - if (documented.has(name)) break; } - } + ts.forEachChild(node, find); + }; + ts.forEachChild(fenceFile, find); } - ts.forEachChild(node, visit); - }; - ts.forEachChild(sourceFile, visit); + } } counters.documented = documented.size; - // Real call sites of those symbols, outside the file that declares them. + // Real call sites of the symbols those fences call. Read from parsed source, so + // a call written inside another comment is not one of them. + const wanted = new Set([...documented.values()].map((entry) => entry.symbol)); const callSites = new Map(); for (const [rel, sourceFile] of parsed) { const visit = (node) => { - if (ts.isCallExpression(node) && ts.isIdentifier(node.expression)) { + if (ts.isCallExpression(node) && ts.isIdentifier(node.expression) && wanted.has(node.expression.text)) { const name = node.expression.text; - const target = documented.get(name); - if (target && target.file !== rel) { - const bindings = bindingsFor(node); - const slots = new Map(); - for (const [slot, expression] of slotsOf(node)) slots.set(slot, inline(expression, bindings)); - if (!callSites.has(name)) callSites.set(name, []); - callSites.get(name).push({ file: rel, slots }); - counters.callSites += 1; - } + const bindings = bindingsFor(node); + const slots = new Map(); + for (const [slot, expression] of slotsOf(node)) slots.set(slot, inline(expression, bindings)); + if (!callSites.has(name)) callSites.set(name, []); + callSites.get(name).push({ file: rel, slots }); + counters.callSites += 1; } ts.forEachChild(node, visit); }; @@ -420,9 +516,11 @@ export function analyze(root) { } const findings = []; + const raw = []; const compared = []; - for (const [name, doc] of documented) { - const sites = callSites.get(name) ?? []; + for (const doc of documented.values()) { + const name = doc.symbol; + const sites = (callSites.get(name) ?? []).filter((site) => site.file !== doc.file); if (!sites.length) continue; for (const [slot, docExpression] of doc.slots) { const delegating = new Map(); @@ -441,13 +539,12 @@ export function analyze(root) { counters.compared += 1; for (const [readerName, users] of delegating) { const reader = readers.get(readerName); - compared.push({ symbol: name, slot, reader: readerName, users: users.length }); + compared.push({ file: doc.file, symbol: name, slot, reader: readerName, users: users.length }); if (callsFunction(docExpression, readerName)) continue; const handSpelled = reader.rungs.find((rung) => canonicalContains(docExpression, rung)); if (!handSpelled) continue; const key = `${doc.file}::${name}::${slot}`; - if (KNOWN_HAND_SPELLINGS.has(key)) continue; - findings.push({ + const finding = { key, file: doc.file, symbol: name, @@ -457,11 +554,13 @@ export function analyze(root) { readerFile: reader.file, handSpelled, users, - }); + }; + raw.push(finding); + if (!KNOWN_HAND_SPELLINGS.has(key)) findings.push(finding); } } } - return { findings, counters, compared }; + return { findings, raw, stale: staleExemptions(raw), counters, compared }; } /** @@ -496,7 +595,7 @@ if (invokedDirectly) { return index > -1 ? process.argv[index + 1] : null; }; const root = resolve(argOf('--root') ?? resolve(scriptDir, '..')); - const { findings, counters, compared } = analyze(root); + const { findings, stale, counters, compared } = analyze(root); if (process.argv.includes('--list')) { for (const entry of compared) { @@ -515,6 +614,15 @@ if (invokedDirectly) { process.exit(1); } + if (stale.length) { + console.error( + `x ${stale.length} KNOWN_HAND_SPELLINGS row(s) name a doc comment this gate no longer finds:\n` + + stale.map((key) => ` ${key}`).join('\n') + + '\n\nThe defect the row waives is gone, so the row is now a live waiver for nothing. Delete it.', + ); + process.exit(1); + } + if (!findings.length) { console.log( `OK ${counters.documented} documented symbol(s), ${counters.callSites} call site(s), ` + From f0c425a1a04f94d8c590b8831311ad0c603e062b Mon Sep 17 00:00:00 2001 From: os-sam Date: Sat, 5 Sep 2026 19:29:16 +0000 Subject: [PATCH 3/3] fix(tooling): narrow the rungsOf fixture through ts.isVariableStatement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tsconfig.scripts.json` type-checks `scripts/**/*.ts`, and `Statement` has no `declarationList` — TS2339 at the fixture, caught by `pnpm type-check:scripts`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3 --- scripts/__tests__/check-doc-example-shared-reader.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/__tests__/check-doc-example-shared-reader.test.ts b/scripts/__tests__/check-doc-example-shared-reader.test.ts index aca7428e3..fc16d6456 100644 --- a/scripts/__tests__/check-doc-example-shared-reader.test.ts +++ b/scripts/__tests__/check-doc-example-shared-reader.test.ts @@ -352,7 +352,10 @@ describe('check-doc-example-shared-reader — the parts', () => { it('reads the alternatives a reader resolves between as its rungs', () => { const source = `const x = dataConfig?.provider === 'object' ? dataConfig.object : schema?.objectName;`; const sf = ts.createSourceFile('r.ts', source, ts.ScriptTarget.Latest, true); - const expression = sf.statements[0].declarationList.declarations[0].initializer; + const statement = sf.statements[0]; + if (!ts.isVariableStatement(statement)) throw new Error('fixture must parse to a variable statement'); + const expression = statement.declarationList.declarations[0].initializer; + if (!expression) throw new Error('fixture must carry an initializer'); expect(rungsOf(expression)).toEqual( expect.arrayContaining([