Add a zero-tolerance TypeDoc documentation gate#58
Conversation
Prepare for a zero-tolerance TypeDoc documentation gate: document every
declaration reachable from the package entry point, including one-line
JSDoc on each zod schema field surfaced through the fixture builders
and the transform-derived fields at their `return { … }` sites.
Tag the zod schema constants with documented `/** … @internal */`
blocks — they are validation seams whose meaning is carried by the
named output types. Convert the `GitHub*` output aliases from
`type … = z.infer<…>` to `interface … extends z.infer<…> {}` so
TypeDoc renders builder return types as references to the documented
interfaces instead of expanding the inferred structural type (the type
is unchanged; TypeDoc cannot preserve `z.infer` alias references), and
give each `build*Fixture` an explicit return-type annotation.
Add `docs-check` to `make all` between `typecheck` and `lint` (after `typecheck` so the generated GraphQL types exist): TypeDoc's `notDocumented` validation over the package entry point (`src/index.ts`, `resolve` strategy, `emit: "none"`, validation warnings as errors), configured by `typedoc.json` and run through `bun run docs:check`. The gate requires 100% documentation of the public surface, reports the qualified name of each undocumented declaration, and writes no documentation artefacts. CI already runs `make all`, so the gate needs no workflow changes.
|
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
Validation
WalkthroughAdd a TypeDoc documentation check, document exported APIs and fixture schemas, configure warning enforcement without artefacts, and include ChangesTypeDoc documentation gate
Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 3 warnings)
✅ Passed checks (16 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11ae114060
ℹ️ 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".
| # Zero-tolerance documentation gate: TypeDoc's notDocumented validation over | ||
| # the package entry point (typedoc.json). Runs after typecheck so the | ||
| # generated GraphQL types already exist. Emits no documentation artefacts. | ||
| docs-check: |
There was a problem hiding this comment.
When make docs-check is run after make clean, or make -j all allows this target to start before typecheck, src/__generated__/resolvers-types.ts is absent even though the TypeDoc entry-point import graph reaches src/graphql/resolvers.ts, which imports that file. Merely placing docs-check after typecheck in all does not enforce ordering for parallel Make, so the documentation gate can fail before generation finishes; declare typecheck or generate as an actual prerequisite.
Useful? React with 👍 / 👎.
|
@coderabbitai review |
✅ Action performedReview finished.
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/store/entities/ref.ts (1)
66-83: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep
qualifiedNameunqualified during normalization.When both values are supplied,
qualifiedNamebecomes the fullrefsuch as
refs/heads/main.refStoreKey()and the derivednode_idthen use that prefixed
value, so equivalent branch fixtures can receive different identities. Normalize the
prefix intorefonly and retainmain/v1.2.3asqualifiedName.Proposed normalization
- const qualifiedName = ref.ref ?? ref.qualifiedName; - const fullRef = qualifiedName.startsWith('refs/') - ? qualifiedName - : `${defaultRefPrefix(ref.object.type)}${qualifiedName}`; + const rawRef = ref.ref ?? ref.qualifiedName; + const fullRef = rawRef.startsWith('refs/') + ? rawRef + : `${defaultRefPrefix(ref.object.type)}${rawRef}`; + const qualifiedName = fullRef.replace(/^refs\/(?:heads|tags)\//, '');Add regression tests covering both branch and tag inputs.
🤖 Prompt for 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. In `@src/store/entities/ref.ts` around lines 66 - 83, Update the normalization transform around rawRef, fullRef, and qualifiedName so qualifiedName remains the unqualified branch or tag name while ref receives the fully prefixed path. Ensure refStoreKey and derived node_id use the unqualified qualifiedName, and add regression tests for both branch and tag inputs.
🤖 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 `@docs/development.md`:
- Around line 36-39: Update the documentation-gate description in
docs/development.md to identify src/index.ts as TypeDoc’s package entry point,
while referring to typedoc.json only as the configuration that selects it.
Preserve the existing notDocumented validation, zero-tolerance warning behavior,
and no-artifact details.
- Around line 33-35: Update the contributor workflow guidance in
docs/development.md so the documented manual gate includes bun run docs:check,
or replace it with make all as the sole contributor gate. Keep the documented
command order and existing workflow description consistent with the new TypeDoc
check.
In `@Makefile`:
- Around line 22-34: The Makefile currently documents a serial order that is not
preserved by parallel Make execution. Add an explicit typecheck prerequisite to
docs-check, encode any required downstream ordering such as lint, and update
docs/development.md to describe the guaranteed dependency rather than relying on
prerequisite-list order; apply the Makefile change at Makefile lines 22-34 and
the documentation clarification at docs/development.md lines 33-35.
In `@src/index.ts`:
- Around line 22-41: The exported TypeScript API lacks compile-time coverage.
Add a type-only test covering GitHubSimulatorArgs.extend.extendRouter in
src/index.ts, plus the exported GitHubBranch alias in
src/store/entities/branch.ts, GitHubCommit in src/store/entities/commit.ts, and
GitHubPullRequest in src/store/entities/pull-request.ts; verify these symbols
can be imported and used with their intended types without runtime tests.
In `@src/store/entities.ts`:
- Around line 56-61: Update the documentation comment for the minimal GitHub
user fixture validator to state that name is derived from login only when
missing, then a missing contact email is derived from the normalized name;
clarify that a caller-provided name is preserved.
In `@src/store/entities/blob.ts`:
- Around line 34-41: Add executable JSDoc examples to both exported key helpers:
in src/store/entities/blob.ts lines 34-41, document blob coordinates and the
resulting canonical key; in src/store/entities/branch.ts lines 50-55, document
branch coordinates and the resulting key. Keep the examples consistent with each
helper’s documented input and output format.
In `@src/store/entities/branch.ts`:
- Line 48: Add a concise JSDoc description immediately above the GitHubBranch
interface declaration, documenting its public API purpose while leaving the
interface and githubBranchSchema unchanged.
In `@src/store/entities/issue.ts`:
- Around line 76-81: The timestamp fields in both schemas accept arbitrary
strings despite documenting ISO 8601 values. Update issue.ts fields created_at,
updated_at, and closed_at, and repository.ts fields pushed_at, updated_at, and
created_at to enforce Zod datetime validation while preserving optional/default
and nullable behavior; add malformed-input tests covering these fields in both
schemas.
In `@src/store/entities/repository.ts`:
- Around line 32-39: Update the documentation comment for the repository fixture
normalization schema to accurately state that REST URL fields are accepted when
provided, rather than claiming omitted fields are filled in. Keep the
descriptions of generated id, node_id, and derived full_name unchanged.
- Around line 220-249: Replace the z.string() validators for the four security
status fields in the repository schema with z.enum(['enabled', 'disabled']) so
only documented values are accepted. Update the schema tests to cover both
allowed statuses and rejection of other strings, preserving the existing
defaults of {status: 'enabled'}.
---
Outside diff comments:
In `@src/store/entities/ref.ts`:
- Around line 66-83: Update the normalization transform around rawRef, fullRef,
and qualifiedName so qualifiedName remains the unqualified branch or tag name
while ref receives the fully prefixed path. Ensure refStoreKey and derived
node_id use the unqualified qualifiedName, and add regression tests for both
branch and tag inputs.
🪄 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: 5f87d8ba-055b-4843-94c2-faae450af742
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.locksrc/__generated__/resolvers-types.tsis excluded by!**/__generated__/**
📒 Files selected for processing (17)
Makefiledocs/development.mdpackage.jsonsrc/index.tssrc/store/builders.tssrc/store/entities.tssrc/store/entities/blob.tssrc/store/entities/branch.tssrc/store/entities/commit.tssrc/store/entities/installation.tssrc/store/entities/issue.tssrc/store/entities/organization.tssrc/store/entities/pull-request.tssrc/store/entities/ref.tssrc/store/entities/repository.tssrc/store/entities/shared.tstypedoc.json
| `make all` runs `check-fmt`, `typecheck`, `docs-check`, `lint`, `test`, and | ||
| `spelling` in the repository's preferred order. The `lint` target runs the | ||
| `biomejs` and `oxlint` sub-targets. The `docs-check` target (`bun run |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Include the TypeDoc gate in the normal contributor workflow.
The manual gate immediately above still lists only formatting, linting, typechecking, and
tests. Add bun run docs:check or make make all the sole documented contributor gate
so local guidance does not bypass the new zero-tolerance check.
Triage: [type:docstyle]
🤖 Prompt for 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.
In `@docs/development.md` around lines 33 - 35, Update the contributor workflow
guidance in docs/development.md so the documented manual gate includes bun run
docs:check, or replace it with make all as the sole contributor gate. Keep the
documented command order and existing workflow description consistent with the
new TypeDoc check.
| docs:check`) is the zero-tolerance documentation gate: TypeDoc's | ||
| `notDocumented` validation over the package entry point (`typedoc.json`), | ||
| requiring a JSDoc block on every declaration in the public surface, treating | ||
| warnings as errors, and emitting no documentation artefacts. Zod schema |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Name the actual TypeDoc entry point.
typedoc.json is the configuration file; src/index.ts is the package entry point
configured by it. Rewrite this sentence so the documentation does not describe the
configuration file as the entry point.
Triage: [type:docstyle]
🤖 Prompt for 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.
In `@docs/development.md` around lines 36 - 39, Update the documentation-gate
description in docs/development.md to identify src/index.ts as TypeDoc’s package
entry point, while referring to typedoc.json only as the configuration that
selects it. Preserve the existing notDocumented validation, zero-tolerance
warning behavior, and no-artifact details.
| all: check-fmt typecheck docs-check lint test spelling | ||
|
|
||
| check-fmt: | ||
| bunx @biomejs/biome check --linter-enabled=false --assist-enabled=false . | ||
|
|
||
| typecheck: | ||
| bun run check:types | ||
|
|
||
| # Zero-tolerance documentation gate: TypeDoc's notDocumented validation over | ||
| # the package entry point (typedoc.json). Runs after typecheck so the | ||
| # generated GraphQL types already exist. Emits no documentation artefacts. | ||
| docs-check: | ||
| bun run docs:check |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make the TypeDoc ordering guarantee explicit.
make -j can start docs-check before typecheck completes, so the documented
“after typecheck” order is not guaranteed. Add the prerequisite dependency in
Makefile, and either encode the subsequent lint dependency or document that the
order applies only to serial Make execution.
Makefile#L22-L34: adddocs-check: typecheckand any required downstream ordering.docs/development.md#L33-L35: describe the guaranteed dependency rather than relying
on prerequisite-list order.
As per coding guidelines, changes involving ordering or parallelism must make the
execution model explicit.
📍 Affects 2 files
Makefile#L22-L34(this comment)docs/development.md#L33-L35
🤖 Prompt for 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.
In `@Makefile` around lines 22 - 34, The Makefile currently documents a serial
order that is not preserved by parallel Make execution. Add an explicit
typecheck prerequisite to docs-check, encode any required downstream ordering
such as lint, and update docs/development.md to describe the guaranteed
dependency rather than relying on prerequisite-list order; apply the Makefile
change at Makefile lines 22-34 and the documentation clarification at
docs/development.md lines 33-35.
Source: Coding guidelines
| type FoundationRouter = Parameters< | ||
| NonNullable<Parameters<typeof createFoundationSimulationServer>[0]['extendRouter']> | ||
| >[0]; | ||
|
|
||
| /** Configuration options accepted by {@link simulation}. */ | ||
| export type GitHubSimulatorArgs = { | ||
| /** Seed data validated against `githubInitialStoreSchema` before use. */ | ||
| initialState?: GitHubInitialStore; | ||
| /** Base URL the simulated GitHub REST and GraphQL APIs are served from. */ | ||
| apiUrl?: string; | ||
| /** OpenAPI schema file or inline schema used to generate REST handlers. */ | ||
| apiSchema?: SchemaFile | string; | ||
| /** Hooks for extending the simulated store, REST handlers, and router. */ | ||
| extend?: { | ||
| /** Additional store slices merged into the GitHub simulation store. */ | ||
| extendStore?: GitHubExtendStoreInput; | ||
| /** Builds extra OpenAPI-backed handlers from the extended store. */ | ||
| openapiHandlers?: (simulationStore: ExtendedSimulationStore) => SimulationHandlers; | ||
| /** Registers extra routes on the underlying foundation router. */ | ||
| extendRouter?: (router: FoundationRouter, simulationStore: ExtendedSimulationStore) => void; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files | rg '(^|/)(test|tests|spec|__tests__|type|types|compile).*|(\.test|\.spec)\.(ts|tsx|js|jsx)$|tsconfig.*\.json$|package\.json$|src/index\.ts$|src/store/entities/(branch|commit|pull-request)\.ts$'
printf '\n== Search for type/compile-time tests ==\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \
'\b(expectTypeOf|assertType|tsd|dtslint|//\s*`@ts-expect-error`|type-level|compile-time|satisfies\b|as const\b)' .
printf '\n== Search for references to the affected exported types ==\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' \
'GitHubBranch|GitHubCommit|GitHubPullRequest|GitHubSimulatorArgs|extendRouter|openapiHandlers|extendStore' src test tests __tests__ .Repository: leynos/simulacat-core
Length of output: 30909
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== package.json ==\n'
cat -n package.json | sed -n '1,220p'
printf '\n== tsconfig files ==\n'
for f in $(git ls-files '*tsconfig*.json'); do
printf '\n--- %s ---\n' "$f"
cat -n "$f" | sed -n '1,220p'
done
printf '\n== likely test files ==\n'
git ls-files | rg '(^|/)(test|tests|__tests__)/|(\.test|\.spec)\.(ts|tsx|js|jsx)$|\.test-d\.ts$|\.d\.ts$|\.d\.test\.ts$'Repository: leynos/simulacat-core
Length of output: 6089
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in src/index.ts src/store/entities/branch.ts src/store/entities/commit.ts src/store/entities/pull-request.ts; do
printf '\n== %s ==\n' "$f"
cat -n "$f" | sed -n '1,260p'
doneRepository: leynos/simulacat-core
Length of output: 24181
Add a compile-time test for the exported TypeScript API. Cover GitHubSimulatorArgs.extend.extendRouter in src/index.ts and the exported GitHubBranch, GitHubCommit, and GitHubPullRequest aliases with a type-only test file.
📍 Affects 4 files
src/index.ts#L22-L41(this comment)src/store/entities/branch.ts#L48-L48src/store/entities/commit.ts#L137-L138src/store/entities/pull-request.ts#L184-L185
🤖 Prompt for 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.
In `@src/index.ts` around lines 22 - 41, The exported TypeScript API lacks
compile-time coverage. Add a type-only test covering
GitHubSimulatorArgs.extend.extendRouter in src/index.ts, plus the exported
GitHubBranch alias in src/store/entities/branch.ts, GitHubCommit in
src/store/entities/commit.ts, and GitHubPullRequest in
src/store/entities/pull-request.ts; verify these symbols can be imported and
used with their intended types without runtime tests.
Source: Coding guidelines
| /** | ||
| * Validates and normalizes a minimal GitHub user fixture, deriving a display | ||
| * name and contact email from `login` when they are omitted. | ||
| * | ||
| * @internal | ||
| */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the email fallback description.
State that the schema derives name from login only when missing, then derives a missing email from the normalised name. A caller-provided name is not replaced by login.
- * name and contact email from `login` when they are omitted.
+ * name from `login` when omitted, then derives a missing contact email
+ * from the normalised name.🤖 Prompt for 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.
In `@src/store/entities.ts` around lines 56 - 61, Update the documentation comment
for the minimal GitHub user fixture validator to state that name is derived from
login only when missing, then a missing contact email is derived from the
normalized name; clarify that a caller-provided name is preserved.
| /** | ||
| * Derives the canonical store key for a Git blob, preferring `path` over | ||
| * `sha` when both are present. | ||
| * | ||
| * @param blob Blob coordinate parts containing owner, repo, path, and sha. | ||
| * @returns The canonical key in `owner/repo:reference` format. | ||
| * @throws Error when neither `path` nor `sha` is provided. | ||
| */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add examples to both exported key helpers.
The new JSDoc describes output formats but omits executable usage examples.
src/store/entities/blob.ts#L34-L41: add an@exampleshowing blob coordinates and the returned key.src/store/entities/branch.ts#L50-L55: add an@exampleshowing branch coordinates and the returned key.
As per coding guidelines, function documentation should include clear examples showing usage and outcome.
📍 Affects 2 files
src/store/entities/blob.ts#L34-L41(this comment)src/store/entities/branch.ts#L50-L55
🤖 Prompt for 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.
In `@src/store/entities/blob.ts` around lines 34 - 41, Add executable JSDoc
examples to both exported key helpers: in src/store/entities/blob.ts lines
34-41, document blob coordinates and the resulting canonical key; in
src/store/entities/branch.ts lines 50-55, document branch coordinates and the
resulting key. Keep the examples consistent with each helper’s documented input
and output format.
Source: Coding guidelines
| }); | ||
|
|
||
| export type GitHubBranch = z.infer<typeof githubBranchSchema>; | ||
| export interface GitHubBranch extends z.infer<typeof githubBranchSchema> {} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File map ==\n'
git ls-files src/store/entities/branch.ts src/store/entities | sed 's#^`#-` #'
printf '\n== Outline ==\n'
ast-grep outline src/store/entities/branch.ts --view expanded || true
printf '\n== Relevant references ==\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' \
'GitHubBranch|buildBranchFixture|githubBranchSchema|`@internal`|TypeDoc|typedoc|documentation' \
src package.json .github docs . 2>/dev/null | sed -n '1,220p'
printf '\n== Target file ==\n'
wc -l src/store/entities/branch.ts
cat -n src/store/entities/branch.ts | sed -n '1,220p'
printf '\n== buildBranchFixture definition/usages ==\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' \
'function buildBranchFixture|const buildBranchFixture|buildBranchFixture\s*[:=]|export .*buildBranchFixture|buildBranchFixture\(' \
src test tests . 2>/dev/null | sed -n '1,240p'Repository: leynos/simulacat-core
Length of output: 27902
Document GitHubBranch with JSDoc. Add a short description above src/store/entities/branch.ts:48; the interface is part of the public API surface, so the TypeDoc notDocumented gate will flag it even though githubBranchSchema is @internal.
🤖 Prompt for 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.
In `@src/store/entities/branch.ts` at line 48, Add a concise JSDoc description
immediately above the GitHubBranch interface declaration, documenting its public
API purpose while leaving the interface and githubBranchSchema unchanged.
| /** ISO 8601 timestamp recording when the issue was created. */ | ||
| created_at: z.string().optional().default(defaultTimestamp), | ||
| /** ISO 8601 timestamp recording when the issue was last updated. */ | ||
| updated_at: z.string().optional().default(defaultTimestamp), | ||
| /** ISO 8601 timestamp recording when the issue was closed, or `null` while open. */ | ||
| closed_at: z.string().nullable().optional(), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Align the timestamp contracts across both schemas.
The new public documentation promises ISO 8601 timestamps, while both schemas accept
arbitrary strings. Enforce .datetime() and add malformed-input tests, or weaken both
descriptions to match the actual validators.
src/store/entities/issue.ts#L76-L81: aligncreated_at,updated_at, andclosed_at.src/store/entities/repository.ts#L57-L71: alignpushed_at,updated_at, andcreated_at.
As per coding guidelines, validate I/O boundaries, including persisted data, with Zod.
📍 Affects 2 files
src/store/entities/issue.ts#L76-L81(this comment)src/store/entities/repository.ts#L57-L71
🤖 Prompt for 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.
In `@src/store/entities/issue.ts` around lines 76 - 81, The timestamp fields in
both schemas accept arbitrary strings despite documenting ISO 8601 values.
Update issue.ts fields created_at, updated_at, and closed_at, and repository.ts
fields pushed_at, updated_at, and created_at to enforce Zod datetime validation
while preserving optional/default and nullable behavior; add malformed-input
tests covering these fields in both schemas.
Source: Coding guidelines
| /** | ||
| * Validates and normalizes a minimal GitHub repository fixture, filling in | ||
| * the wide set of GitHub REST URLs, counters, and security settings that | ||
| * callers omit. During transformation, `id` and `node_id` are generated when | ||
| * absent, and `full_name` is derived from `owner`/`name`. | ||
| * | ||
| * @internal | ||
| */ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Describe the repository defaults accurately.
The schema description says omitted REST URL fields are filled in, but those fields are
only optional() and remain absent when omitted. Change the wording to say that the
schema accepts URL fields, or add actual defaults.
🤖 Prompt for 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.
In `@src/store/entities/repository.ts` around lines 32 - 39, Update the
documentation comment for the repository fixture normalization schema to
accurately state that REST URL fields are accepted when provided, rather than
claiming omitted fields are filled in. Keep the descriptions of generated id,
node_id, and derived full_name unchanged.
| /** Advanced Security enablement state for the repository. */ | ||
| advanced_security: z | ||
| .object({ | ||
| /** Whether Advanced Security is `enabled` or `disabled`. */ | ||
| status: z.string() | ||
| }) | ||
| .optional() | ||
| .default({status: 'enabled'}), | ||
| /** Secret scanning enablement state for the repository. */ | ||
| secret_scanning: z | ||
| .object({ | ||
| /** Whether secret scanning is `enabled` or `disabled`. */ | ||
| status: z.string() | ||
| }) | ||
| .optional() | ||
| .default({status: 'enabled'}), | ||
| /** Secret scanning push protection enablement state for the repository. */ | ||
| secret_scanning_push_protection: z | ||
| .object({ | ||
| /** Whether secret scanning push protection is `enabled` or `disabled`. */ | ||
| status: z.string() | ||
| }) | ||
| .optional() | ||
| .default({status: 'enabled'}), | ||
| /** Secret scanning non-provider pattern detection enablement state for the repository. */ | ||
| secret_scanning_non_provider_patterns: z | ||
| .object({ | ||
| /** Whether non-provider pattern secret scanning is `enabled` or `disabled`. */ | ||
| status: z.string() | ||
| }) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Enforce the documented security-status values.
The four status fields are documented as enabled or disabled, but each uses
z.string(). Use z.enum(['enabled', 'disabled']), or document these as free-form
strings and test the accepted contract.
As per coding guidelines, validate I/O boundaries with Zod and add tests for behavioural
invariants.
🤖 Prompt for 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.
In `@src/store/entities/repository.ts` around lines 220 - 249, Replace the
z.string() validators for the four security status fields in the repository
schema with z.enum(['enabled', 'disabled']) so only documented values are
accepted. Update the schema tests to cover both allowed statuses and rejection
of other strings, preserving the existing defaults of {status: 'enabled'}.
Source: Coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/development.md (1)
26-31: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winInclude the TypeDoc gate in the normal contributor gate.
The listed manual gate still runs only formatting, linting, typechecking, and tests. Add
bun run docs:checkafter typechecking, or makemake allthe sole documented contributor gate.Triage: [type:docstyle]
🤖 Prompt for 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. In `@docs/development.md` around lines 26 - 31, Update the “normal contributor gate” in the development documentation to include the TypeDoc validation command `bun run docs:check` immediately after `bun check:types`, while preserving the existing formatting, linting, typechecking, and test steps.
♻️ Duplicate comments (5)
docs/development.md (1)
35-38: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winName
src/index.tsas the package entry point.
typedoc.jsonconfigures TypeDoc; it is not the package entry point. Rewrite the sentence to identifysrc/index.tsas the entry point and retaintypedoc.jsonas its configuration.Triage: [type:docstyle]
🤖 Prompt for 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. In `@docs/development.md` around lines 35 - 38, Update the documentation gate description to identify src/index.ts as the package entry point, while retaining typedoc.json solely as the TypeDoc configuration reference. Preserve the existing explanation of notDocumented validation and public-surface JSDoc requirements.src/store/entities/issue.ts (1)
76-81: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winEnforce the documented timestamp contract across both schemas.
The fields are documented as ISO 8601 timestamps, but arbitrary strings still pass validation. Apply
.datetime()while preserving each field’s existing optional, default, and nullable semantics, then add malformed-input tests.
src/store/entities/issue.ts#L76-L81: validatecreated_at,updated_at, andclosed_at.src/store/entities/repository.ts#L57-L71: validatepushed_at,updated_at, andcreated_at.As per coding guidelines, validate I/O boundaries with Zod.
🤖 Prompt for 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. In `@src/store/entities/issue.ts` around lines 76 - 81, Apply Zod .datetime() validation to created_at, updated_at, and closed_at in src/store/entities/issue.ts:76-81, preserving their current optional, default, and nullable behavior. Apply the same validation to pushed_at, updated_at, and created_at in src/store/entities/repository.ts:57-71, then add tests confirming malformed timestamps are rejected at both schema boundaries.Source: Coding guidelines
src/store/entities/repository.ts (2)
220-249: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winEnforce the documented security-status values.
Replace all four
status: z.string()validators withz.enum(['enabled', 'disabled']), preserve the existing defaults, and add acceptance/rejection tests.As per coding guidelines, validate I/O boundaries with Zod.
🤖 Prompt for 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. In `@src/store/entities/repository.ts` around lines 220 - 249, In the repository schema, update the status validators for advanced_security, secret_scanning, secret_scanning_push_protection, and secret_scanning_non_provider_patterns from unrestricted strings to z.enum(['enabled', 'disabled']). Preserve each existing enabled default and add tests confirming both values are accepted and any other status is rejected at the Zod I/O boundary.Source: Coding guidelines
32-39: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStop claiming that omitted URLs are filled.
Only counters and security settings receive defaults; the URL fields remain optional and absent when omitted. Rewrite this description to state that caller-supplied URLs are accepted, or implement actual URL defaults.
🤖 Prompt for 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. In `@src/store/entities/repository.ts` around lines 32 - 39, Update the documentation for the repository fixture normalizer near its exported function or class to remove the claim that omitted GitHub REST URLs are populated. State that caller-supplied URL fields are preserved or accepted as optional, while retaining the documented defaults for counters and security settings.Makefile (1)
22-34: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winEncode the TypeDoc execution dependency.
The documented order is not guaranteed under
make -j. Makedocs-checkdepend ontypecheck, and make downstream lint ordering explicit if TypeDoc must complete before lint. Update the documentation to describe the guaranteed dependency rather than prerequisite-list order.
Makefile#L22-L34: add the required Make dependency chain.docs/development.md#L33-L35: document the enforced dependency, not merely the preferred order.As per coding guidelines, changes involving ordering or parallelism must make the execution model explicit.
🤖 Prompt for 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. In `@Makefile` around lines 22 - 34, Make the Makefile dependency graph explicit: update docs-check to depend on typecheck, and add any required dependency ensuring lint runs only after docs-check completes. In docs/development.md, describe this enforced dependency chain rather than relying on prerequisite-list order. Apply the Makefile change at Makefile lines 22-34 and update the corresponding documentation at docs/development.md lines 33-35.Source: Coding guidelines
🤖 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 `@src/store/entities.ts`:
- Around line 93-101: Update the full seeded store description in the
documentation above the validation/normalization implementation to include
issues alongside commits and pull requests, accurately reflecting the schemas
parsed by the full-store flow.
In `@src/store/entities/issue.ts`:
- Around line 56-61: The fallback issue ID generation using
ENTITY_ID_OFFSETS.ISSUE plus issue.number is not instance-wide unique across
repositories. Replace it with a repository-aware or global allocator, or require
callers to supply unique IDs, while preserving the documented uniqueness
contract for id. Add a regression test covering identical issue numbers in
different repositories.
In `@src/store/entities/pull-request.ts`:
- Around line 173-176: Update the JSDoc comments for the base and head fields in
the pull request entity mapping to state that missing owner and repo values
default to the owning repository, rather than claiming normalization to it.
Leave the normalizePullRequestRef calls unchanged.
- Around line 115-116: Update the documentation for the pull-request `id` field
in the entity schema to describe the current deterministic fallback as the
configured pull-request offset plus `number`; do not claim uniqueness across
repositories unless store-wide allocation and duplicate validation are
implemented.
In `@src/store/entities/ref.ts`:
- Around line 60-61: The sha schema in the ref entity must enforce the
documented SHA-1 format rather than accepting arbitrary non-empty strings.
Update the Zod validation for sha to require exactly 40 hexadecimal characters,
and add fixture tests covering valid and invalid hashes at the I/O boundary.
In `@src/store/entities/repository.ts`:
- Around line 52-53: Update the full_name field documentation and normalization
contract in the repository entity schema to accurately reflect that the
transform always derives it from owner and name, or change the transform to
preserve a caller-supplied value. Ensure tests cover the selected behavior and
reference the full_name normalization logic.
---
Outside diff comments:
In `@docs/development.md`:
- Around line 26-31: Update the “normal contributor gate” in the development
documentation to include the TypeDoc validation command `bun run docs:check`
immediately after `bun check:types`, while preserving the existing formatting,
linting, typechecking, and test steps.
---
Duplicate comments:
In `@docs/development.md`:
- Around line 35-38: Update the documentation gate description to identify
src/index.ts as the package entry point, while retaining typedoc.json solely as
the TypeDoc configuration reference. Preserve the existing explanation of
notDocumented validation and public-surface JSDoc requirements.
In `@Makefile`:
- Around line 22-34: Make the Makefile dependency graph explicit: update
docs-check to depend on typecheck, and add any required dependency ensuring lint
runs only after docs-check completes. In docs/development.md, describe this
enforced dependency chain rather than relying on prerequisite-list order. Apply
the Makefile change at Makefile lines 22-34 and update the corresponding
documentation at docs/development.md lines 33-35.
In `@src/store/entities/issue.ts`:
- Around line 76-81: Apply Zod .datetime() validation to created_at, updated_at,
and closed_at in src/store/entities/issue.ts:76-81, preserving their current
optional, default, and nullable behavior. Apply the same validation to
pushed_at, updated_at, and created_at in src/store/entities/repository.ts:57-71,
then add tests confirming malformed timestamps are rejected at both schema
boundaries.
In `@src/store/entities/repository.ts`:
- Around line 220-249: In the repository schema, update the status validators
for advanced_security, secret_scanning, secret_scanning_push_protection, and
secret_scanning_non_provider_patterns from unrestricted strings to
z.enum(['enabled', 'disabled']). Preserve each existing enabled default and add
tests confirming both values are accepted and any other status is rejected at
the Zod I/O boundary.
- Around line 32-39: Update the documentation for the repository fixture
normalizer near its exported function or class to remove the claim that omitted
GitHub REST URLs are populated. State that caller-supplied URL fields are
preserved or accepted as optional, while retaining the documented defaults for
counters and security settings.
🪄 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: 5f87d8ba-055b-4843-94c2-faae450af742
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.locksrc/__generated__/resolvers-types.tsis excluded by!**/__generated__/**
📒 Files selected for processing (17)
Makefiledocs/development.mdpackage.jsonsrc/index.tssrc/store/builders.tssrc/store/entities.tssrc/store/entities/blob.tssrc/store/entities/branch.tssrc/store/entities/commit.tssrc/store/entities/installation.tssrc/store/entities/issue.tssrc/store/entities/organization.tssrc/store/entities/pull-request.tssrc/store/entities/ref.tssrc/store/entities/repository.tssrc/store/entities/shared.tstypedoc.json
| /** | ||
| * Validates and normalizes the full seeded initial store: users, | ||
| * installations, organizations, repositories, branches, blobs, refs, | ||
| * commits, and pull requests. During transformation, installations without | ||
| * an `id` are allocated one, and any organization lacking a matching | ||
| * installation account is granted a generated installation. | ||
| * | ||
| * @internal | ||
| */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
List issues in the full-store description.
The schema also parses issues, so the documentation is incomplete while calling this the “full” seeded store.
- * installations, organizations, repositories, branches, blobs, refs,
- * commits, and pull requests.
+ * installations, organizations, repositories, branches, blobs, refs,
+ * commits, issues, and pull requests.🤖 Prompt for 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.
In `@src/store/entities.ts` around lines 93 - 101, Update the full seeded store
description in the documentation above the validation/normalization
implementation to include issues alongside commits and pull requests, accurately
reflecting the schemas parsed by the full-store flow.
| /** Unique numeric identifier for the issue across the whole simulated instance. */ | ||
| id: z.number().optional(), | ||
| /** The GraphQL global node identifier for the issue. */ | ||
| node_id: z.string().optional(), | ||
| /** Issue number, unique within the owning repository. */ | ||
| number: z.number().int().positive(), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make generated issue IDs unique across repositories.
The fallback ENTITY_ID_OFFSETS.ISSUE + issue.number collides for issue #1 in multiple repositories, although the documentation promises instance-wide uniqueness. Allocate IDs from a repository-aware/global allocator, or require callers to provide unique IDs, and add a regression test.
Also applies to: 94-95
🤖 Prompt for 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.
In `@src/store/entities/issue.ts` around lines 56 - 61, The fallback issue ID
generation using ENTITY_ID_OFFSETS.ISSUE plus issue.number is not instance-wide
unique across repositories. Replace it with a repository-aware or global
allocator, or require callers to supply unique IDs, while preserving the
documented uniqueness contract for id. Add a regression test covering identical
issue numbers in different repositories.
| /** Unique numeric identifier for the pull request across the whole simulated instance. */ | ||
| id: z.number().optional(), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not claim global pull-request ID uniqueness without enforcing it.
The fallback uses only number plus a constant, so pull request number 1 can produce the same id in multiple repositories. Either implement store-wide allocation and duplicate validation, or describe the current deterministic fallback accurately.
-/** Unique numeric identifier for the pull request across the whole simulated instance. */
+/** Numeric identifier, defaulting to the configured pull-request offset plus `number`. */Also applies to: 165-166
🤖 Prompt for 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.
In `@src/store/entities/pull-request.ts` around lines 115 - 116, Update the
documentation for the pull-request `id` field in the entity schema to describe
the current deterministic fallback as the configured pull-request offset plus
`number`; do not claim uniqueness across repositories unless store-wide
allocation and duplicate validation are implemented.
| /** Base (target) branch reference, normalized to the owning repository. */ | ||
| base: normalizePullRequestRef(pullRequest.base, pullRequest.owner, pullRequest.repo), | ||
| /** Head (source) branch reference, normalized to the owning repository. */ | ||
| head: normalizePullRequestRef(pullRequest.head, pullRequest.owner, pullRequest.repo), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Describe reference normalization accurately.
normalizePullRequestRef preserves supplied owner and repo values; it only defaults missing values to the pull request repository. Replace the current wording accordingly.
-/** Base (target) branch reference, normalized to the owning repository. */
+/** Base (target) branch reference; missing owner and repo default to the owning repository. */
-/** Head (source) branch reference, normalized to the owning repository. */
+/** Head (source) branch reference; missing owner and repo default to the owning repository. */🤖 Prompt for 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.
In `@src/store/entities/pull-request.ts` around lines 173 - 176, Update the JSDoc
comments for the base and head fields in the pull request entity mapping to
state that missing owner and repo values default to the owning repository,
rather than claiming normalization to it. Leave the normalizePullRequestRef
calls unchanged.
| /** SHA-1 hash of the object the ref points to. */ | ||
| sha: z.string().trim().min(1), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Enforce the documented object-hash contract.
z.string().trim().min(1) accepts arbitrary values despite documenting sha as a SHA-1 hash. Enforce the intended hash format with Zod, or describe the field as an opaque identifier, and add valid/invalid fixture tests.
As per coding guidelines, validate I/O boundaries with Zod.
🤖 Prompt for 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.
In `@src/store/entities/ref.ts` around lines 60 - 61, The sha schema in the ref
entity must enforce the documented SHA-1 format rather than accepting arbitrary
non-empty strings. Update the Zod validation for sha to require exactly 40
hexadecimal characters, and add fixture tests covering valid and invalid hashes
at the I/O boundary.
Source: Coding guidelines
| /** Fully qualified `owner/name` identifier, derived automatically if omitted. */ | ||
| full_name: z.string().optional().default(''), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Describe full_name normalisation accurately.
The transform always replaces repo.full_name with ${repo.owner}/${repo.name}, even when input supplies another value. State that the field is always derived, or preserve the caller-supplied value and test the chosen contract.
🤖 Prompt for 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.
In `@src/store/entities/repository.ts` around lines 52 - 53, Update the full_name
field documentation and normalization contract in the repository entity schema
to accurately reflect that the transform always derives it from owner and name,
or change the transform to preserve a caller-supplied value. Ensure tests cover
the selected behavior and reference the full_name normalization logic.
2 participants
Summary
This branch adds a zero-tolerance TypeDoc documentation gate to
make all. TypeDoc'snotDocumentedvalidation runs over the packageentry point (
src/index.ts,resolvestrategy) withemit: "none"and validation warnings treated as errors: every declaration in the
public surface must carry a JSDoc comment. The gate reports the
qualified name of each undocumented declaration, writes no
documentation artefacts, and fails on a single omission. CI already
runs
make all, so it reaches the gate with no workflow changes.The first commit documents the surface (the initial inventory was 1,450
warnings, almost all zod-inference noise); the second wires the gate.
Review walkthrough
for the gate's configuration, then the
Makefile
and package.json:
docs-checksits betweentypecheckandlintinmake all, aftertypecheckso the generated GraphQL types exist.src/store/entities/
carry documented
/** … @internal */blocks, and each schema fieldsurfaced through the fixture builders now has a one-line JSDoc at its
field site (including the transform-derived fields at their
return { … }sites incommit.ts,issue.ts,pull-request.ts,and
ref.ts).src/store/entities/
and src/store/builders.ts:
the
GitHub*output aliases becomeinterface … extends z.infer<…> {}(type unchanged) and thebuild*Fixturehelpers gain explicit return-type annotations.TypeDoc cannot preserve
z.inferalias references, so without thisthe builders' return types expand into hundreds of undocumentable
structural properties.
documents the simulator argument surface, and
docs/development.md
documents the local command and updates the gate flowchart.
Validation
make all(check-fmt, typecheck,docs-check, lint via Biome andOxlint, test, spelling): exit 0.
InitialStatealias makesbun run docs:checkexit 4 naming thesymbol; restoring it returns the gate to green.
docs:checkrun emits no files.Notes
docs:checkprints nine cosmetic "unused@param" warnings for thedestructured-parameter helpers in
src/store/keys.ts: the Oxlintdf12
require-public-jsdocrule requires one@paramper bindingname, while TypeDoc binds a destructured parameter to a single name.
These are not validation warnings, so they do not fail the gate; the
Oxlint rule was kept authoritative.
Replace docstring-coverage with a zero-tolerance TypeDoc gate df12-build#62, Replace the docstring audit with a zero-tolerance TypeDoc gate dakar#5, and Add a zero-tolerance TypeDoc documentation gate digitalpuddle#47).