docs(repo): fix documentation accuracy, consistency, and completeness#5
Merged
zrosenbauer merged 4 commits intomainfrom Mar 12, 2026
Merged
docs(repo): fix documentation accuracy, consistency, and completeness#5zrosenbauer merged 4 commits intomainfrom
zrosenbauer merged 4 commits intomainfrom
Conversation
Address 24 verified issues found during comprehensive documentation review: HIGH: Fix CLI dependency references (yargs → @kidd-cli/core) across architecture, CLI concepts, and CLAUDE.md. Fix wrong GitHub org in contributor getting-started (joggr → joggrdocs). Correct default sort order in auto-discovery guide (alpha → discovery order). Add missing `icon` top-level config field to reference. Fix `outline` type missing `'deep'` in frontmatter reference. Fix throwing example in error handling standards. Fix PR title examples using undocumented scopes. MEDIUM: Complete architecture module tables and sync pipeline steps. Fix generate command description (add "icon"). Add required/optional indicators to WorkspaceGroup table. Fix Entry content type. Add pnpm build step to contributor getting-started. Resolve CI test contradiction. Standardize CLI invocation in deployment guide. Add missing scopes to git-commits. Add release type to CLAUDE.md. Add _tag discriminator to error examples. Add Result testing guidance. Fix type-fest recommendation. Resolve isNil contradiction. Add PR template. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRepository documentation and contributing guides updated: new PR template added; CLI tooling references switched to Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contributing/standards/typescript/errors.md (1)
204-214:⚠️ Potential issue | 🟡 MinorAdd
_tagto the remainingParseErrorexamples.This section still returns
{ type, message }without the discriminator introduced earlier, so the page now documents two differentParseErrorshapes.Suggested fix
function parse(json: string): Result<Data, ParseError> { if (!json) { - return [{ type: 'parse_error', message: 'Empty input' }, null] + return [{ _tag: 'ParseError', type: 'parse_error', message: 'Empty input' }, null] } try { return [null, JSON.parse(json)] } catch { - return [{ type: 'parse_error', message: 'Invalid JSON' }, null] + return [{ _tag: 'ParseError', type: 'parse_error', message: 'Invalid JSON' }, null] } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contributing/standards/typescript/errors.md` around lines 204 - 214, The ParseError examples in the parse function return objects like { type: 'parse_error', message: ... } but omit the discriminator introduced earlier; update both error returns in function parse to include the _tag discriminator property (e.g., add _tag: 'ParseError' or whatever discriminator string your types use) so the returned error shape matches the ParseError type used elsewhere; modify the two error objects inside parse (the empty-input return and the catch return) to include _tag alongside type and message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Line 64: Replace the vague line "Config validated at boundaries" with an
explicit requirement that configuration must be validated using Zod at module
boundaries; update the AGENTS.md wording so it reads something like "Validate
configuration with Zod at module boundaries" and ensure any guidance or examples
in the surrounding text reference using Zod schemas and parse/parseAsync (or
safeParse) for validation so readers know exactly which library and API to use.
In `@contributing/concepts/architecture.md`:
- Around line 21-26: Update the table row for `@zpress/kit` to accurately
describe its public surface: replace "re-exports everything + provides the CLI
bin" with text that lists the actual public entry points and CLI surface, e.g.,
state that it exposes the main entry point ('.'), a './config' entry point, and
provides a CLI via the package's bin rather than exporting the CLI as an
importable module; edit the `@zpress/kit` row in the package table accordingly.
In `@contributing/standards/typescript/testing.md`:
- Around line 170-176: The test "should return error for missing file" asserts
the wrong ConfigError variant; update the assertion in the test that calls
loadConfig('/missing') so the error._tag/type matches a missing-file scenario
(e.g., set type: 'missing_file') or rename the test to reflect a missing-field
case—locate the test by the it(...) description and the loadConfig('/missing')
call and change the expected type from 'missing_field' to the correct variant
(or adjust the test name) so the assertion semantically matches the scenario.
In `@docs/references/cli.md`:
- Around line 114-120: Update the example command text so it follows the page's
canonical npx convention: replace the standalone "zpress generate" example with
"npx zpress generate" (i.e., change the occurrence of the string "zpress
generate" in the CLI example block to "npx zpress generate") so the docs
consistently use the npx form.
---
Outside diff comments:
In `@contributing/standards/typescript/errors.md`:
- Around line 204-214: The ParseError examples in the parse function return
objects like { type: 'parse_error', message: ... } but omit the discriminator
introduced earlier; update both error returns in function parse to include the
_tag discriminator property (e.g., add _tag: 'ParseError' or whatever
discriminator string your types use) so the returned error shape matches the
ParseError type used elsewhere; modify the two error objects inside parse (the
empty-input return and the catch return) to include _tag alongside type and
message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 760b84bc-cfad-43b3-baef-106066a6d0d8
📒 Files selected for processing (19)
.github/PULL_REQUEST_TEMPLATE.mdAGENTS.mdcontributing/concepts/architecture.mdcontributing/concepts/cli.mdcontributing/guides/developing-a-feature.mdcontributing/guides/getting-started.mdcontributing/standards/git-commits.mdcontributing/standards/git-pulls.mdcontributing/standards/typescript/errors.mdcontributing/standards/typescript/testing.mdcontributing/standards/typescript/types.mdcontributing/standards/typescript/utilities.mddocs/guides/auto-discovery.mddocs/guides/deployment.mddocs/guides/sidebar-icons.mddocs/guides/workspaces.mddocs/references/cli.mddocs/references/configuration.mddocs/references/frontmatter.md
- Restore explicit Zod validation requirement in AGENTS.md - Tighten @zpress/kit surface-area description in architecture docs - Fix mismatched error type in testing standard example - Fix invalid `npx zpress` references to use `npx @zpress/kit` or bare `zpress` Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
contributing/standards/typescript/testing.md (1)
170-176:⚠️ Potential issue | 🟡 MinorAlign the failure example with the asserted error variant.
This example still mixes a missing-file scenario (
loadConfig('/missing')) with amissing_fieldassertion. Either change the fixture/scenario to a malformed config missing a required field, or update the expectedtypeto the missing-file variant so the example stays semantically correct.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contributing/standards/typescript/testing.md` around lines 170 - 176, The test "should return error for missing field" currently calls loadConfig('/missing') but asserts a 'missing_field' error; update the scenario so they match by changing the fixture path passed to loadConfig to one that represents a config missing a required field (e.g., '/missing_field' or '/malformed_missing_field') so the assertion on _tag 'ConfigError' and type 'missing_field' is correct; locate the test case by the it(...) block and the loadConfig call and update the fixture name accordingly (alternatively, if you prefer to keep the '/missing' fixture, change the asserted type to 'missing_file').
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contributing/concepts/architecture.md`:
- Around line 175-189: Update the documented pipeline steps in
contributing/concepts/architecture.md to match the actual execution order in
packages/core/src/sync/index.ts: reorder "Public asset copying" to appear early
(with the code's step number 0/1 as shown in the file), move "Planning page
discovery" to its real position after Collect pages/Generate home (reflecting
the code's placement at line ~90), add the missing "Write bare-bones README"
step (label it with the code's internal step number as used in sync/index.ts),
and adjust all step numbers to the internal sequence used in the code comments
(0, 1, 1.25, 1.5, 2, 2.1, 2.5, 3, 5, 6, 7, 8) so the list matches the
implementation.
---
Duplicate comments:
In `@contributing/standards/typescript/testing.md`:
- Around line 170-176: The test "should return error for missing field"
currently calls loadConfig('/missing') but asserts a 'missing_field' error;
update the scenario so they match by changing the fixture path passed to
loadConfig to one that represents a config missing a required field (e.g.,
'/missing_field' or '/malformed_missing_field') so the assertion on _tag
'ConfigError' and type 'missing_field' is correct; locate the test case by the
it(...) block and the loadConfig call and update the fixture name accordingly
(alternatively, if you prefer to keep the '/missing' fixture, change the
asserted type to 'missing_file').
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac94f92b-9481-4f39-b93e-d17417a35036
📒 Files selected for processing (5)
AGENTS.mdcontributing/concepts/architecture.mdcontributing/standards/typescript/testing.mddocs/guides/deployment.mddocs/references/cli.md
🚧 Files skipped from review as they are similar to previous changes (1)
- AGENTS.md
Reorder pipeline steps to match actual execution in sync/index.ts: - Move public asset copying before manifest loading (step 3) - Move planning page discovery after generate home (step 11) - Merge source map building into copy pages step - Add missing write README step (step 16) Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
docs/(public-facing) andcontributing/(internal) identified 28 issues; 24 non-subjective issues fixed across 19 filesChanges
HIGH severity fixes
@clack/promptsreferences with@kidd-cli/coreinarchitecture.md,cli.md, andAGENTS.md(CLAUDE.md symlink)joggr/zpress→joggrdocs/zpressin contributor getting-started'alpha'sort; clarified discovery order is the defaulticonconfig field: AddedZpressConfig.iconto configuration referenceoutlinetype: Added missing'deep'variant to main tableJSON.parse()in try/catch in the "Never Throw" sectionruntime/config→packages/coreto match documented scopesMEDIUM severity fixes
generatecommand: Added "icon" to description ("banner, logo, and icon SVG assets")contenttype to include sync function variantpnpm buildbefore verification stepnpx zpresspackages/uiandpackages/zpressscopes; addedreleasecommit type_tagdiscriminator to all examples; notedHandlerResultas plannedtype-festnot currently installedisNilcontradiction (standalone vs predicate context).github/PULL_REQUEST_TEMPLATE.mdAdditional improvements
auto-discovery.mdandsidebar-icons.mdworkspaces.md@clack/promptspurpose to "Sync engine prompts & output"src/lib/watcher.ts)Testing
pnpm format:fixpassesRelated Issues
N/A — proactive documentation quality improvement
Summary by CodeRabbit
New Features
Documentation
Chores