Add a zero-tolerance TypeDoc documentation gate - #436
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary
WalkthroughAdd zero-output TypeDoc validation for workspace packages, document exported entities, and include ChangesQuality gates and lint tooling
Sequence Diagram(s)sequenceDiagram
participant CI
participant Makefile
participant pnpm
participant TypeDoc
CI->>Makefile: run docs-check
Makefile->>pnpm: run docs:check
pnpm->>TypeDoc: load three configurations
TypeDoc-->>pnpm: return validation status
pnpm-->>Makefile: complete docs-check
Makefile-->>CI: return gate status
Change: Feature Merge Risk: 🟡 Moderate · up to The new quality gates can either miss action files when traversal fails or fail the repository lint job on the added suppressions. These issues should be corrected before merge. Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (14 passed)
Full details: Testing (Overall)Explanation The new CI contract tests do not fully guard the changed quality gates. Resolution Strengthen TypeDoc checks each public sign, Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddfe8b22a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend-pwa/src/api/fetcher.ts`:
- Around line 178-181: Update the documentation for customFetchParsedSafe to
clarify that safeParse returns a result only for schema-validation failures,
while transport errors and non-2xx responses from customFetch still reject.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e59c38df-dc1c-4ca9-b25a-e92025e65a60
⛔ Files ignored due to path filters (2)
packages/types/dist/src/user.d.tsis excluded by!**/dist/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
Makefilefrontend-pwa/src/api/client.tsfrontend-pwa/src/api/fetcher.tsfrontend-pwa/src/app/App.tsxfrontend-pwa/typedoc.jsonpackage.jsonpackages/tokens/build/validate-contrast.jspackages/tokens/src/utils/resolve-token.jspackages/tokens/tsconfig.typedoc.jsonpackages/tokens/typedoc.jsonpackages/types/src/user.tspackages/types/typedoc.json
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/cuprum(auto-detected)leynos/rstest-bdd(auto-detected)leynos/pg-embed-setup-unpriv(auto-detected)leynos/ortho-config(auto-detected)
7e62a49 to
1b83985
Compare
85041a3 to
43a73e8
Compare
43a73e8 to
993dfe5
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
a81f4b3 to
b5cd206
Compare
…e head (#485) * Clear the three Ruff findings the Python gate was hiding `make lint-python` has been printing these while exiting zero, because the Makefile's global `.ONESHELL` gives a recipe its last command's status and Ruff is not the last command. The next commit closes that hole, so these have to go first or it lands on a red tree. `cache_ownership_test.py` imported `pathlib` and never used it. `tool_installation_test.py` conflated two claims in one assertion: that the Makefile invokes `cargo binstall` at all, and that every invocation carries `--force`. Split, an empty list now says "no invocation found" rather than reporting a missing flag. The other finding is a long line, rewrapped. * Give the Makefile tooling contracts their own TMPDIR Two contracts asserted the command doubles saw an empty `TMPDIR`. Nothing in the Makefile sets or clears `TMPDIR`, so that assertion was reading the ambient environment: it held on GitHub's runners, which set none, and failed on any developer machine that does. The suite reported a Makefile defect that did not exist. The fixture now owns a `TMPDIR` under the test's own temporary directory and both contracts assert the tools receive exactly that. The claim becomes "the Makefile passes the caller's TMPDIR through untouched", which is about the Makefile rather than about whoever ran the suite, and it holds with `TMPDIR` unset, set to the fixture's value, or set to anything else. * Expect both cargo-audit ignores in the audit contract #473 added `--ignore RUSTSEC-2026-0258` to `CARGO_AUDIT_IGNORES` without updating this suite, so its exact-match assertion has been failing since. `make test-frontend` runs `pnpm run test` before `pnpm run test:workspaces` and the Makefile's global `.ONESHELL` hands the target the last command's status, so the failure never reached the gate. Both ignores stay. Each is justified in the Makefile's header comment with its own review date, and RUSTSEC-2026-0258 has no patched release for actix-http's h2 dependency. The expected command is now a named constant used by all three assertions. The two that used `toContain` matched only the first ignore, so dropping the second passed them; against the full command a dropped suppression fails all three. * Retire the three expired Redocly lint exceptions All three carried `review by: 2026-08-31`, which passed a week ago. `scripts/check_redoc_ignore.py` has been reporting them, but it is not the last command in the `lint-openapi` recipe and the Makefile's global `.ONESHELL` hands the target the last command's status, so the gate stayed green over an expired exception. The review the date was demanding finds all three obsolete rather than due for renewal: - `no-empty-servers` on `#/servers`: the specification now declares a server, `/`, described as relative to the deployment base URL. - `operation-4xx-response` on both health probes: each documents a 405 alongside 200 and 503, so each carries a 4xx response. Redocly lints the specification cleanly with no exceptions at all, and emptying `servers` still trips `no-empty-servers`, so the rule is live and the exception was simply spent. The two remaining warnings are the unused component schemas tracked in #484. The `spec/openapi.json` key stays, because the recipe asserts the specification has an entry here, and the file gains a header recording what the exceptions were and why they went. * Make a failing recipe line fail its Make target `.ONESHELL` is a global special target: GNU make ignores its prerequisite list, so the declaration naming `prepare-pg-worker` documents which recipe needed one-shell recipes but turns them on for the whole file. Every multi-line recipe reaches the shell as a single script, and under make's default `.SHELLFLAGS` of `-c` that script's status is its last command's status. Every earlier failure is discarded. The symptom is the worst one a gate can have. Run 33939820204 went green with `make lint-python` printing Ruff findings, and `make lint-openapi` passed for a week over an expired review-by annotation, because in each recipe the failing tool was not the last command. The four commits before this one clear everything the flag surfaces, so it lands on a green tree. `.SHELLFLAGS := -ec` makes the shell abort at the first failing command. The `c` stays: make passes the recipe to the shell as a command string. No recipe in this Makefile relies on a non-zero intermediate status; every tolerated failure is already inside an `if`, a `||`, or a captured status, and no recipe line carries make's `-` prefix. A recipe that later needs a command to be allowed to fail should say so itself rather than have this flag weakened for it. `makefile_failure_propagation_test.py` holds the contract. It builds a scratch Makefile from the repository's own prologue lines plus a probe target whose first line fails and last line succeeds, drives GNU make over it, and asserts the target fails. A companion test removes the `.SHELLFLAGS` line from that same prologue and asserts the identical probe passes, so the first test's verdict is attributable to the flag and not to a mistake in the probe. A third pins the assumption the whole thing rests on, that `.ONESHELL` applies to targets it does not name. Deleting the flag fails the contract; weakening it to `-c` fails both the declaration check and the behavioural probe, the latter reporting exit 0 with the probe's failing line having run. * Fail pipelines at the head as well as recipes at the line `-ec` alone was half a fix. It aborts at the first failing command, which catches a tool that is not the recipe's last line, but a pipeline still reports its LAST stage's status, so a failure at the head is discarded exactly as before. That is not hypothetical here. This Makefile pipes into the tool that does the checking: `spelling` feeds `git ls-files` into typos, and `lint-actions` feeds `find` into yamllint and actionlint. A head that dies produces an empty list, and the gate passes having examined nothing. `-o pipefail` gives a pipeline its first failing stage's status. Neither option covers the other, so both are needed and neither can be dropped. The contract now drives two probes. `earlier-line` fails on a line that is not the last; `pipeline-head` fails in a pipeline's first stage while its last stage succeeds. Each is mutation-proved against the half-measure that masks it: with `-ec` alone `pipeline-head` passes, and with `-o pipefail -c` alone `earlier-line` passes. A third case asserts the copied prologue still declares `.ONESHELL`, without which make would run each line in its own shell and both probes would pass under any flags. The guide no longer offers make's `-` line prefix as an escape hatch for a recipe that needs a tolerated failure. Under `.ONESHELL` it applies to the first recipe line only, so on any later line it silently does nothing. Reported by the review round on lille #345. * Address the review round on the failure-propagation contract Match `--force` as a whole argument. The comment above the assertion already warned that `--force-exclude` elsewhere in the Makefile would satisfy a search for the flag, and then the assertion did exactly that search. Splitting the line into arguments makes the check match the rule it serves: rewriting the invocation to `--force-exclude` now fails the contract, where before it passed. Give `_make` a single-line docstring, per the numpy convention for private helpers, and move its rationale to a comment above the definition. Caption the settings table. The documentation style guide puts the caption below the table, so it goes there rather than above it. * Guard each linter in lint-actions on its own Two shapes in this recipe lose a command's status without help, and both are invisible in the output. `action-validator` runs once per composite action, so an earlier iteration's failure is replaced by the last one's status; the loop is dead today with no `.github/actions` directory and goes live the day one appears. The workflows branch runs `find | xargs yamllint` and then `find | xargs actionlint` inside one `if`, where the second's status replaces the first's. `.SHELLFLAGS`'s `-e` covers both today. Each invocation carries `|| exit 1` anyway: the guard states the intent where the reader meets it, and it survives a future change to the flag that these shapes would not. Three contracts hold it, because no single one can. A static assertion requires every linter invocation in the recipe to carry a guard, and dropping any one of the three fails its own case. A behavioural pair drives the real recipe with a failing first linter and a passing second, with the all-passing case beside it so a failure is attributable. A probe in `makefile_failure_propagation_test.py` switches `-e` off with `set +e` and measures the guard alone, since no behavioural test can tell the guard and the flag apart while both are present; its unguarded twin must still pass, or the probe is measuring something else. Reported by env-wildside's loop audit. * Move the lint-actions gate logic out of the recipe The `|| exit 1` guards were interim. The estate's scripting standards say gate logic of this size does not belong in a recipe at all, and this recipe had two shapes that lose a command's status: `action-validator` in a loop over composite actions, and `find | xargs yamllint` followed by `find | xargs actionlint` in one `if`. Guards paper over both; a script removes them. `scripts/lint_actions.py` builds the list of invocations first and runs them in one loop, so "the first failure wins" is a property of four lines rather than of control flow spread through a shell fragment. It reports the tool the reader cares about rather than the one that ran it: yamllint arrives through `uvx`, and naming `uvx` would send them to the wrong place. The recipe is now one command. `scripts/tests/test_lint_actions.py` places a failing tool first, a failing tool last, and no failing tool at all, with cmd-mox supplying the executables. The plan's ordering is asserted separately, since without it "the first tool failed" is not a claim the failure tests can make. The contract over the recipe changes shape with it: instead of asserting each invocation carries a guard, it asserts the recipe runs exactly one command besides its tool checks, that the command invokes the script with the yamllint pin, and that it contains no `while`, `;`, `&&` or `||`, any of which would reintroduce the defect. The shell probes for the guarded shapes retire; the ordering they stood in for is now tested directly. Two things this turned up, both recorded in the guide for the next script. cuprum 0.1.0 has no `Catalogue.from_programs`, which is what `docs/scripting-standards.md` shows; a `ProgramCatalogue` is built from a `ProjectSettings` and passed to `sh.make`, and program names are the `Program` type rather than plain strings. And cmd-mox needs an interpreter that can import it on the shim's PATH, which a layered `uv run --with` environment is not: the shim hangs rather than failing. `test-lint-actions` builds a materialized virtual environment instead, as `typecheck-python` does, which is also why the new dependencies join the typecheck set. * Run the lint-actions script tests in CI The new suite reached `make test` but not the workflow, which invokes the individual test targets rather than the aggregate. It therefore passed locally and never ran on a pull request, which is the quietest way for a test to be useless. The contract matches the command rather than the step name and asserts the step is unconditional; deleting the step fails it. * Stream the linters' output through cuprum's echo The script relayed both captured streams after each tool finished, which made a slow linter look hung and reordered its output against the script's own messages. `run_sync(echo=True)` mirrors them as the tool writes them, and capture stays on so a failure can still quote stderr. The skip notices are flushed for the same reason: cuprum's echo writes to the file descriptor directly while print buffers, so without a flush the notice arrived after the output it introduces. The guide records that this is the repository's first cuprum script, that the three plumbum scripts are the migration the standard anticipates, and the two upstream reports this work produced: leynos/concordat#154 for the standard's examples not matching cuprum's shipped API, and leynos/cmd-mox#249 for the shim hanging rather than failing. * Correct the account of why the cmd-mox tests use a venv The comment and the guide both blamed the layered `uv run --with` environment, saying the shim could not import cmd_mox there. That is wrong. The shim's shebang resolves `python3` to the layered interpreter, both `CMOX_IPC_SOCKET` and `CMOX_IPC_TIMEOUT` reach the child, and the same invocation passes repeatedly; two other repositories run cmd-mox under layered environments without trouble. What is real is a stall that appears under load, always alongside the server logging `IPC received malformed JSON`, with the shim ignoring its own `CMOX_IPC_TIMEOUT` while it waits. Three consecutive runs stalled while the machine was busy compiling; three later runs of the same file passed. The target still builds a materialized virtual environment, because that is where the suite has been stable and `typecheck-python` already needs one, but the reason is now stated as the observation it is rather than as a cause I had not tested. leynos/cmd-mox#249 carries the disproof and asks for a bounded wait, which is the fix worth having whatever the race is. * Address the review round on the lint-actions script Require the CI step's whole `run` value to be the command. A per-line search is satisfied by a step that wraps the command in `if false; then ... fi` and runs nothing, which is worse than no contract: this one exists because the suite it guards would otherwise never run on a pull request. The `if` key assertion stays, since the two catch different things, a YAML-level condition and a shell-level one. Rewriting the step into a false guard now fails the contract. Require GNU Make in the failure-propagation contract. `.ONESHELL` is a GNU extension, so under a non-GNU make the probe targets run line by line and the whole contract passes while measuring nothing; a green run would have been indistinguishable from a correct one. The helper prefers `gmake`, falls back to `make`, and accepts a candidate only when `--version` begins with `GNU Make`. Give the script's public interfaces their NumPy sections: `Parameters` and `Attributes` on `LintError`, `Attributes` on `Invocation`, `Parameters` and `Returns` on `plan`, `Parameters` and `Raises` on `lint`, `Raises` on `main`. The private helpers keep single-line summaries with their rationale in adjacent comments, which is the same instruction read the other way. Annotate `TMPDIR_SENTINEL`. * Separate discovery from planning, and test the entry point Three pre-merge rows, all fair. `plan` was presented as a query but walked the filesystem, and an unreadable directory yielded nothing exactly as an absent one does. That is this pull request's own defect one level down: a gate that reports success having examined nothing. `discover` now does the walking and raises `DiscoveryError` when a directory exists but cannot be listed, while an absent directory still yields nothing, because a repository with no composite actions is normal. `plan` is left a pure function of its inputs, which is what lets the ordering be asserted without a filesystem. The listing failure is caught both when it starts and while it is consumed, since `rglob` can raise at either point depending on the failure. Nothing tested `main`. Every test drove `lint`, so a script that detected a failure and exited zero would have passed all of them. Two cases now drive the entry point: a failing linter must exit with that linter's own status and name it on stderr, and an all-passing run must exit quietly. Two property tests cover the invariants stated over an unbounded set, which fixed fixtures cannot show: `plan` orders any number of surfaces the same way, and `lint` stops at whichever invocation fails. The second drives the real loop with a substituted runner rather than reimplementing it, and patches through a context because Hypothesis will not reuse a function-scoped fixture across examples. * Split the surface-ordering property test CodeScene flagged `test_plan_orders_every_surface_the_same_way` as a Complex Method: it built the repository, computed the expected order, and asserted both the tool sequence and the manifests' sort order in one body. The repository construction and the expected sequence move to helpers, and the sort-order claim becomes its own property. They are separate invariants: one says which tools run and in what order, the other says the manifests within a run do not depend on what the filesystem returned. A test asserting both told the reader which had failed only by line number. * Assert the gate cannot be skipped as well as not rewritten Requiring the step's whole `run` value closed two holes and left a third: a condition skips the step with its `run` untouched, so the contract passes while the gate never executes. `if: false` does it, and so does an ordinary looking `github.event_name == 'push'`, which skips the gate on exactly the event it exists to cover. A gate holds only when three things hold together, each of which defeats the others alone: the workflow triggers on `pull_request`, exactly one step has the command as its whole `run` value, and neither the job nor that step carries an `if` key. The condition is asserted on the key's presence, never on its value. A condition need not be falsy to skip the gate, and comparing against the string "false" would pass its own mutation anyway: YAML parses `false` to a boolean whose string form is `False`. Eight mutations, all failing the contract: `if: false` on the step, `if: false` on the job, a push-only condition on each, the command wrapped in `if false; then ... fi`, `|| true` appended, the command changed, and the `pull_request` trigger removed. Reported by the round on repovec-appliance #105. * Report a directory the gate cannot read, rather than skipping it `discover` raised `DiscoveryError` on a listing failure and its test passed, but neither did anything: `Path.rglob` and `Path.glob` suppress the `OSError` a denied directory raises and yield nothing instead. A subtree the gate could not read was therefore indistinguishable from a subtree with no manifests, and the run reported success having examined neither. The test passed because it patched `rglob` to raise, which real `rglob` never does. It measured the error path of a function that has no error path. `_manifests_under` now walks with `Path.walk(on_error=...)`, which reports what `glob` discards, and `_workflows_in` uses `iterdir`, which raises rather than suppressing. The test creates a directory it genuinely cannot enter and asserts the failure surfaces; restoring the `rglob` implementation fails it, where before it passed. A companion test pins the behaviour this rests on, so if a future Python makes `rglob` propagate, the walk becomes belt and braces and the test says so by failing rather than by silently becoming redundant. Both errors now derive from a `LintActionsError` base, so a caller can catch this gate's expected failures without catching every `Exception`. Reported by the round on #436, where this script appears in the diff. * Reject continue-on-error, and hold the lint-actions target's shape `_assert_gate_runs_unconditionally` rejected a condition but not `continue-on-error`, which I dropped when the per-gate assertions moved into the shared helper. The two are different routes to the same place: a condition skips the gate, `continue-on-error` runs it and discards the verdict, and both leave a green pull request the gate never held. Both keys are now rejected on the job and on the step, by presence rather than value, and each of the two new mutations fails the contract. Two contracts hold `test-lint-actions` itself. `make test` must gather it, so a contributor gets the answer CI will give them rather than discovering it after pushing. And the recipe must materialize a virtual environment rather than layer one, because cmd-mox's shim hangs under `uv run --with` instead of failing, and a hang is the worst thing to leave for the next person to diagnose. Dropping the prerequisite and switching the recipe to `uv run --with` each fail their own case. * Document the traversal error handler `interrogate` requires every definition to carry a docstring, including a nested one, and the `on_error` handler had none. CI caught it because this branch is the one that makes `lint-python` report its own verdict; I had run Ruff directly and skipped the target, so I missed the tier behind it.
b5cd206 to
4af9147
Compare
Prepare for a zero-tolerance TypeDoc documentation gate across the JavaScript/TypeScript workspaces: document the fetch mutators and query keys in `frontend-pwa/src/api`, the root `App` component, the zod user schema fields in `@app/types` (field-site comments so the inferred output types carry them), the token-reference resolver in `@app/tokens`, and the contrast validator's `PackageJson` typedef (converted to `@property` form so each field can carry a description).
Add `docs-check` to `make all` (and a root `docs:check` script) covering three surfaces, each with its own configuration beside its tsconfig: `frontend-pwa` (`src`, `expand`, excluding the generated Orval client under `src/api/generated` and tests), `packages/types` (`src`, `expand`), and `packages/tokens` (JavaScript via a dedicated `tsconfig.typedoc.json` with `allowJs`, `checkJs: false`). Each runs TypeDoc's `notDocumented` validation with `emit: "none"` and validation warnings treated as errors: 100% documentation of the selected surfaces, qualified names reported for any omission, and no documentation artefacts written. TypeDoc and TypeScript 5.9.2 are added at the workspace root with pnpm.
Wire `docs-check` into pull-request CI and document the gate's maintained surfaces and exclusions. Correct the fetcher and token utility API documentation, and add build-contract coverage for the Make, package-script, CI, and validation-failure paths.
Keep fallible setup and synchronization failures visible at Rust test boundaries so Whitaker can enforce panic placement without suppressions. Correct the specification, workflow, AsyncAPI, local Kubernetes, and Nixie validation paths exposed by the full commit-gate run.
Record the TypeDoc and TypeScript development dependencies in Bun's lockfile so documentation validation no longer rewrites the working tree during its install step.
… change The rebase onto main carried four files that are out of scope for the TypeDoc gate and that main has since resolved on its own. `.github/workflows/dependabot-automerge.yml` gained a duplicated `"on":` mapping because main adopted the same quoting fix independently and the structural merge kept both copies. `backend/src/doc.rs` was left with repeated import lists and a module doc comment spliced into the middle of the file, neither of which compiles. `backend/src/inbound/http/admin_enrichment.rs`, `backend/src/inbound/http/offline.rs` and `spec/openapi.json` dropped the `ToSchema` derives for `ListEnrichmentProvenanceQuery` and `ListOfflineBundlesQuery` so the two unused component schemas would leave the published specification. Redocly reports those as warnings, not errors, and `make lint-openapi` passes on main with them present, so the change is an unrelated specification edit rather than a gate repair. It belongs in its own pull request against the OpenAPI surface. Every file here now matches main exactly.
A caret range on `typedoc` lets a minor release change the gate's verdict without a reviewed commit, so the root manifest now names 0.28.20 exactly and both lockfiles resolve that version. New contract tests assert the shape of the pin and each lockfile's resolution. The CI contract matched the step by name and then read its `run` value. A name is prose: renaming the step, or deleting it beside a similarly named neighbour, satisfied a name-shaped assertion. It now searches every step in the `build` job for one whose command line is `make docs-check`, and asserts that step carries neither an `if` guard nor `continue-on-error`, so the gate is unconditional and fails the job. Both mutations, replacing the command while keeping the name and deleting the step outright, fail the contract. The fixture test proves TypeDoc honours `notDocumented` with warnings treated as errors, but it says nothing about the three real configurations. A parametrized test now asserts each of them sets that policy, emits nothing, and lists at least one required declaration kind. `packages/tokens/typedoc.json` gained the `excludeInternal`, `excludePrivate` and `excludeProtected` settings the other two surfaces already carried, so `@internal` means the same thing everywhere. The developers' guide gains the declaration kinds each surface requires, the pinning rule, a local run recipe, and a worked example of documenting an export. It no longer claims a "qualified declaration names" setting that no configuration key controls.
Ruff wants a blank line after the module docstring and its import block sorted; mdtablefix rewraps the paragraphs this branch added. No wording or assertion changes.
`make lint-python` reported these but still exited zero: the Makefile's global `.ONESHELL` means only the recipe's last command decides the target's status, so Ruff's verdict was discarded. Read from the full output instead. The two subprocess calls now resolve their executable through `shutil.which`, so PATH no longer decides what runs, and each carries the `S603` suppression the resolved-but-still-external call needs. The membership test against `None` and `False` uses a set literal.
TypeDoc runs from `node_modules`, so on a clean checkout `make docs-check` failed with a missing binary rather than a documentation verdict. Naming `deps` as a prerequisite matches `typecheck`, which has the same need.
The three configurations disabled TypeDoc's link validation, so a
`{@link}` naming a symbol that does not exist passed silently. A broken
cross-reference is a documentation defect in the same category as a
missing comment: the reader follows it and lands nowhere.
All three surfaces already pass with `invalidLink`, `invalidPath` and
`rewrittenLink` enabled, so this costs nothing today and holds the line
tomorrow. Pointing `usersQueryKeys.all` at a symbol that does not exist
makes `typedoc --options frontend-pwa/typedoc.json` exit 4.
`notExported` stays off. It reports a different thing, a referenced type
that is not part of the public surface, and that is not what this gate
is for.
The contract matched the literal text of the `all` target's line, so reordering an unrelated prerequisite failed it while saying nothing about docs-check. It now parses the line and checks the prerequisite list. Dropping docs-check fails the contract; reordering the rest does not.
Assert each surface's `requiredToBeDocumented` set whole rather than merely non-empty. That key decides which declaration kinds `validation.notDocumented` can see, so a list that drops `Function` leaves undocumented exported functions sailing through a gate still calling itself zero-tolerance, and a non-empty check cannot tell the difference. The two TypeScript surfaces expect all ten kinds; the tokens surface is JavaScript, so the expectation is the same set less the four kinds that cannot occur there, derived rather than restated. Dropping `Function` from the frontend configuration now fails the contract. Describe `customFetch`'s content-type behaviour as it is. The previous wording implied plain objects and strings were treated alike, when a plain object is serialized and a string is passed through on the assumption that it is already JSON. It also omitted that native body types are sent unchanged with no content type, and that a caller-supplied `Content-Type` is never overwritten.
A static contract over typedoc.json proves the gate is configured. It cannot prove it catches anything: a configuration can name every key a reviewer expects and still admit an undocumented export, because what each key does is TypeDoc's business rather than the configuration's. `documentation_gate_behaviour_test.py` runs the real TypeDoc against the repository's own frontend configuration over a fixture, overriding only the entry point, the TypeScript configuration and the project name. A documented fixture passes and leaves behind only the three files the test wrote, so `emit: "none"` is observed rather than assumed. A missing doc comment, a link to a symbol that does not exist and an unknown block tag each fail with a diagnostic naming them. Clearing the responsible key admits exactly its own fixture and leaves the other two failing, which is what ties a key to a defect rather than to an expectation of one. Measuring that matrix turned up a real hole: an unknown block tag warned and the gate still exited zero, because `treatValidationWarningsAsErrors` promotes validation warnings only. A misspelled tag name passed silently. `treatWarningsAsErrors` closes it, and all three surfaces already pass with it on. The narrower key stays, since it carries the intent on its own if the broader one is ever removed, but it is subsumed today and neither the contract nor the guide pretends otherwise. The static contract gains what it was missing: the validation object compared whole rather than key by key, the entry points and exclusions per surface, the `deps` prerequisite on `docs-check`, and an assertion that no recipe line carries make's `-` prefix. Each is mutation-proved: dropping `deps`, adding a `-` prefix, narrowing an entry point, widening the exclusions and removing `treatWarningsAsErrors` each fail exactly one test. The undocumented fixture gains a module comment explaining its purpose, in line comments rather than a JSDoc block. A block comment there attaches to the declaration as its documentation, which made the export documented and the test it serves pass for the wrong reason.
The cases read only the front-end configuration, which made the other two surfaces' behaviour an assumption. The three are maintained separately, so one can lose a key while the others keep it, and a test reading only the front end would never notice. Each case now runs against every surface's own configuration. Clearing `treatWarningsAsErrors` in `packages/tokens/typedoc.json` alone fails four cases, all of them that surface's, where before it failed none.
The insertion left a 126-column line, which `make markdownlint` rejects under MD013. I pushed 90d162b without seeing that, because I piped the gate through `tail` and read the filter s exit status instead of the gate s.
The contract matched a command line within the step's `run`, so a step wrapping the command in `if false; then ... fi` satisfied it while running nothing, and a condition on the step or the job skipped the gate with the `run` value untouched. Either leaves a green pull request that never ran the gate. It now uses the shared `_assert_gate_runs_unconditionally` helper, which requires three things together: the workflow triggers on `pull_request`, exactly one step has the command as its whole `run` value, and neither the job nor that step carries an `if` key. The condition is rejected by the key's presence rather than by its value, because a condition need not be falsy to skip the gate: `github.event_name == 'push'` skips it on precisely the event it exists to cover. Nine mutations, all failing the contract: `if: false` on the step, `if: false` on the job, a push-only condition on each, the command wrapped in a false guard, `|| true` appended, the command changed, the step deleted, and the `pull_request` trigger removed. Rebased onto #485 at 202a2cd, which adds the helper.
`f"typedoc@{pin}" in text` is an unanchored substring search, so a pin of
`0.28.2` matched the text `typedoc@0.28.20`. The manifest and the lockfiles
could disagree and the contract would still report green, which is exactly
the drift it exists to catch.
The match now ends on a non-version character. Shortening the manifest pin
to `0.28.2` fails both lockfile cases, where before it passed both.
The structural merge joined the docs-check contract to the function below it, leaving one blank line where Ruff wants two. I pushed d30502e without seeing it: I ran the gate sweep and the push in one chain, so the failing format check scrolled past above a successful `cs delta` and the push ran regardless.
4af9147 to
4364365
Compare
Summary
This branch adds a zero-tolerance TypeDoc documentation gate to
make allandpull-request CI. It covers the three maintained JavaScript and TypeScript
surfaces:
frontend-pwa/src,packages/types/src, and the JavaScript tokentooling under
packages/tokens.Each surface keeps its TypeDoc configuration beside its TypeScript
configuration. The root
docs:checkscript chains all three configurations,and
make docs-checkruns the script. Every run usesemit: "none", treatsvalidation warnings as errors, and writes no documentation artefacts. Generated
Orval clients, generated declarations, tests, fixtures, and generated output
remain excluded.
TypeDoc is pinned to 0.28.20 exactly in the root manifest and resolved to that
version in both
pnpm-lock.yamlandbun.lock. A caret range would let aminor release change the gate's verdict without a reviewed commit.
Base
Rebased onto
mainnow that #485 has merged as 9108047. This branch wasstacked on it while it was open; those commits are gone from the diff and the
19 that remain are this pull request's own.
Rebase
Rebased onto
mainat c48ad2b, through #485 while it was open, and ontomainagain at 9108047 once it merged. The pre-rebase head was7e62a49; the branch had been conflicting
since July, so GitHub had stopped computing a merge commit and the
pull_requestworkflows were no longer running against it.The fourth commit, "Repair gate-discovered validation failures", was almost
entirely superseded during the rebase.
mainhas since landed the sameWhitaker helper-panic fixes, the same uv rework in the
Makefile, the samelocal_k8stest isolation, and the same"on":quoting independabot-automerge.yml, all independently. What survived the structuralmerge was damaged: a duplicated
"on":mapping and abackend/src/doc.rscarrying repeated import lists and a module doc comment spliced into the middle
of the file. A commit on top restores those five files to match
mainexactly.What remained after that was an unrelated specification edit dropping the
ToSchemaderives fromListEnrichmentProvenanceQueryandListOfflineBundlesQueryso the two unused component schemas leavespec/openapi.json. Redocly reports those as warnings rather than errors andmake lint-openapipasses onmainwith them present, so that is aspecification cleanup rather than a gate repair. It is tracked as #484 and is
not carried here.
Changes on top of the original branch
shape and each lockfile's resolution of it.
A step name is prose: renaming it, or deleting the step beside a similarly
named neighbour, satisfied a name-shaped assertion. It now searches the
buildjob for a step whose command line ismake docs-check, and assertsthat step carries neither an
ifguard norcontinue-on-error. Bothmutations, replacing the command while keeping the name and deleting the step
outright, fail the contract.
validation.notDocumented, treats warnings as errors, emits nothing, andlists at least one required declaration kind. The fixture test proved TypeDoc
honours the policy but said nothing about the configurations that ship.
packages/tokens/typedoc.jsongained theexcludeInternal,excludePrivateand
excludeProtectedsettings the other two surfaces already carried.docs-checknamesdepsas a prerequisite, so a clean checkout gets adocumentation verdict rather than a missing-binary error.
{@link}naming asymbol that does not exist now fails the gate. All three already passed with
it on. Pointing
usersQueryKeys.allat a symbol that does not exist makestypedoc --options frontend-pwa/typedoc.jsonexit 4.notExportedstaysoff; it reports a different thing.
alltarget's prerequisite list instead ofmatching its whole line, so reordering an unrelated prerequisite no longer
fails a contract that is about
docs-check.pinning rule, a local run recipe, and a worked example of documenting an
export.
Validation
Run from the rebased branch:
make docs-check: all three configurations pass and emit no files.surface's configuration, naming the declaration:
api/client.usersQueryKey,UserIdSchema, andsrc/utils/resolve-token.resolveToken.{@link}at a symbol that doesnot exist, makes the frontend configuration exit 4.
frontend-pwa/src/api/generated/with an undocumented exportdoes not fail the gate, so the Orval exclusion holds.
make test-workflow-contracts: 120 passed.make check-fmt,make typecheck,make markdownlint,make nixie,make spelling,make lint-openapi,make lint-actions,make lint-makefile,make yamllint: pass.Cargo.lockorCargo.tomldelta againstmain.buildandcoverageboth green, with theTypeDoc gate step's log showing all three configurations running.
The four pre-existing failures previously listed here are fixed in #485, which
this branch now sits on, so every gate above is green with no known exceptions.
Notes
Replace docstring-coverage with a zero-tolerance TypeDoc gate df12-build#62, Replace the docstring audit with a zero-tolerance TypeDoc gate dakar#5, Add a zero-tolerance TypeDoc documentation gate digitalpuddle#47,
Add a zero-tolerance TypeDoc documentation gate simulacat-core#58, Add a zero-tolerance TypeDoc documentation gate vibe-coder#110, and
Add a frontend zero-tolerance TypeDoc documentation gate corbusier#156).
References
Summary by Sourcery
Enforce zero-tolerance TypeDoc validation across maintained JavaScript and TypeScript APIs in local aggregate checks and pull-request CI.
New Features:
Enhancements:
make alland unconditional pull-request CI.Build:
docs:checkpackage script andmake docs-checktarget, including dependency installation as a prerequisite.CI:
make docs-checkin the CI build job and add workflow contract coverage for the gate.Documentation:
Tests:
Chores: