Repository navigation
feat-add-create-docs-cli-package-ci - #5
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR adds an 11-phase AI-driven skill for setting up Python API docs in Starlight sites, a compiler to publish the skill across Claude/Cursor/Copilot, a new ChangesPython Documentation Setup Workflow & Release
🎯 4 (Complex) | ⏱️ ~50 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
package.json (1)
11-17:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd missing
docs:pythonscript to root scripts block.
package.jsonis missingscripts["docs:python"], so the Python autodoc command path required by repo standards is incomplete. Please add it with the exact value:"scripts": { "dev": "bun --filter `@abstract-data/playground` dev", "build": "bun --filter `@abstract-data/playground` build", "preview": "bun --filter `@abstract-data/playground` preview", "typecheck": "bun --filter '*' typecheck", + "docs:python": "node scripts/build-python-docs.mjs", "sync-skills": "node scripts/compile-skill.mjs" },As per coding guidelines, "
package.jsonmust includescripts['docs:python']set tonode scripts/build-python-docs.mjs."🤖 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 `@package.json` around lines 11 - 17, Add a new root package.json script entry named scripts["docs:python"] with the exact value "node scripts/build-python-docs.mjs"; update the existing scripts block (which currently includes "dev", "build", "preview", "typecheck", "sync-skills") to include this new key so the repo's Python autodoc command path is present and uses node scripts/build-python-docs.mjs.AGENTS.md (1)
52-52:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale
⏳marker — the CLI scaffolder is delivered by this PR.Line 52 still flags
bun create@abstractdata/docs`` as a future item, butpackages/create-docsadded in this PR is exactly that deliverable. This misleads future AI agents reading `AGENTS.md`.📝 Proposed fix
- - ⏳ **Future:** `bun create `@abstractdata/docs`` CLI scaffolder (separate `create-abstractdata-docs` package). + - ✅ **Round 5 (this PR):** `bun create `@abstractdata/docs`` CLI scaffolder (`@abstractdata/create-docs` package) with 11-phase Python autodoc skill, multi-tool distribution (Claude Code / Cursor / Copilot), and publish CI.🤖 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 `@AGENTS.md` at line 52, Update the AGENTS.md entry that still shows "⏳ **Future:** `bun create `@abstractdata/docs`` CLI scaffolder" to reflect that the CLI scaffolder has been delivered by this PR: remove the ⏳/Future marker, change the wording to a delivered/available state (e.g., "✅ Delivered: `bun create `@abstractdata/docs``"), and optionally reference the new package `packages/create-docs` so readers/agents can locate the implementation.
🤖 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 @.claude/skills/abstract-data-setup/SKILL.md:
- Line 189: Replace the hardcoded "src/" in the interrogate hook args (currently
shown as args: [--fail-under=80, -v, src/]) with the package root/path detected
in Phase 3 (e.g., use the variable produced by your detection step such as
package_root or detected_source_path) so the hook runs against the real module
layout; implement detection by checking for pyproject.toml, setup.py,
requirements.txt, src/<pkg>/__init__.py, or <pkg>/__init__.py and inject that
path into the interrogate args, with a sensible fallback (current directory) if
no package root is found.
In @.github/workflows/release-please.yml:
- Around line 55-74: publish-create-docs lacks a pre-publish quality gate; add a
step before the "Publish" step to run the package test/typecheck in the
packages/create-docs working directory (mirror what publish-starlight-theme
does). Specifically, insert a step named like "Run tests (quality gate)" with
working-directory: packages/create-docs and run either npm test or bun test
(since package.json defines "test": "node --test") so the job fails on
test/typecheck failures before npm publish; ensure NODE_AUTH_TOKEN remains only
on the publish step.
In `@apps/playground/package.json`:
- Around line 9-13: Build currently skips generating Python docs, so update the
npm scripts so the "build" script runs the "docs:python" task before running
"astro check" and "astro build"; modify the "build" entry (and any CI/deploy
steps that call it) to run "npm run docs:python" (or the equivalent script
invocation) first so generated API pages are always up to date when executing
the "build" command that currently references "build" and the "docs:python"
script names.
In `@apps/playground/scripts/build-python-docs.mjs`:
- Around line 127-135: The frontmatter generation uses an unquoted title which
breaks YAML when title contains special characters; update the frontmatter
assembly where the frontmatter constant is built (the lines creating frontmatter
and the template entry `title: ${title}`) to emit a properly quoted and escaped
title (e.g., `title: "${escapedTitle}"`) before calling writeFileSync(outPath,
frontmatter + body); ensure you create an escapedTitle by replacing any existing
double quotes and backslashes in the title string so the produced YAML is valid.
- Around line 79-87: The command string passed to execSync embeds the
user-controlled variable mod unquoted, creating a shell injection risk; update
the call to avoid shell interpolation by using child_process.execFileSync (or
execSync with shell disabled and args array) so that pydoc-markdown is invoked
with explicit args instead of a single interpolated string, passing searchPath
and mod as separate arguments (reference the existing execSync call, the
variables searchPath and mod, and the pydoc-markdown invocation).
In `@apps/playground/scripts/pydoc-markdown.yml`:
- Around line 19-22: The search_path entry in pydoc-markdown.yml currently
contains a hardcoded absolute developer path
(/sessions/zen-affectionate-johnson/mnt/website-auditkit/src) which will break
for other contributors/CI; replace that value with a relative path (e.g., ./src
or ../src as appropriate) or a placeholder like ${PROJECT_SRC} and update README
to document how to set/override search_path (and how to substitute the
placeholder or run pydoc-markdown from the correct working directory); ensure
the config comment is corrected to mention using a relative path or environment
variable instead of claiming an absolute path resolves everywhere.
In `@apps/playground/src/content/docs/api/auditkit_config.md`:
- Around line 39-44: The `@property` signatures for
http_transport_backend_order_tuple and assume_cms_tuple are missing the required
self parameter; update the doc generation or source docstrings so the generated
signatures read as instance properties (include self) — either fix the source
Python docstrings for the properties or adjust the post-processing in
build-python-docs.mjs to insert "self" into the `@property` signatures for
http_transport_backend_order_tuple and assume_cms_tuple to ensure the docs show
instance methods rather than module-level functions.
In `@apps/playground/src/content/docs/api/example_module.md`:
- Around line 12-18: The docs for the function `normalize` conflict with its
name by saying "Clamp"; inspect the actual implementation of `normalize` and
either (A) if it clamps, rename the API and docs to `clamp` (update all
occurrences of `normalize` in docs and examples to `clamp`), or (B) if it
rescales/normalizes, update the description to explain the rescaling behavior
(e.g., map input from its source range into the inclusive [lo, hi] using the
standard normalization/rescaling formula) and adjust examples accordingly;
reference the `normalize` symbol in the codebase (or `clamp` if you choose to
rename) to make the change consistent across docs and examples.
In `@packages/create-docs/bin/cli.js`:
- Around line 76-85: The copyRecursive function currently copies .git
directories into new projects; update copyRecursive (the function named
copyRecursive in packages/create-docs/bin/cli.js) to skip any entry whose name
is '.git' (add '.git' to the existing exclusion check alongside 'node_modules',
'dist', '.astro', 'bun.lock') so that directories named '.git' are not copied
and therefore won't conflict with the later git init step; ensure the check
applies before recursing (i.e., keep the exclusion logic at the start of the
loop).
- Around line 88-103: The code reads and JSON.parse's the template package.json
into pkg (pkgPath, pkg) without error handling; wrap the
JSON.parse(readFileSync(pkgPath, 'utf8')) call in a try/catch, and on catch call
the existing die() helper with a clear, user-friendly message that includes
pkgPath and the parse error (or error.message) so malformed package.json yields
a controlled error instead of an uncaught SyntaxError; ensure subsequent code
(setting pkg.name/version/private, dependency replacement, writeFileSync) only
runs when parsing succeeds.
In `@packages/create-docs/README.md`:
- Line 23: Replace the incorrect package name `@abstractdata/docs-template` in
the README with the actual template location: explain that the CLI copies the
local template bundled inside the `@abstractdata/create-docs` package (the
`template/` directory, copied from `packages/template/` during prepack), e.g.
change the sentence to say "Copies the `template/` files bundled inside the
`@abstractdata/create-docs` package" so users aren't pointed to a non-existent
npm package.
In `@scripts/compile-skill.mjs`:
- Around line 104-111: Remove the redundant replacement in function genericize:
the final .replace(/Use `AskUserQuestion`/g, 'Ask the user') is unnecessary
because the earlier .replace(/Use `AskUserQuestion`(\s+for every choice)?/g,
'Ask the user via your interactive prompt mechanism$1') already matches the bare
"Use `AskUserQuestion`" form; delete that last replace call to avoid duplication
and clarify the intent of genericize.
---
Outside diff comments:
In `@AGENTS.md`:
- Line 52: Update the AGENTS.md entry that still shows "⏳ **Future:** `bun
create `@abstractdata/docs`` CLI scaffolder" to reflect that the CLI scaffolder
has been delivered by this PR: remove the ⏳/Future marker, change the wording to
a delivered/available state (e.g., "✅ Delivered: `bun create
`@abstractdata/docs``"), and optionally reference the new package
`packages/create-docs` so readers/agents can locate the implementation.
In `@package.json`:
- Around line 11-17: Add a new root package.json script entry named
scripts["docs:python"] with the exact value "node
scripts/build-python-docs.mjs"; update the existing scripts block (which
currently includes "dev", "build", "preview", "typecheck", "sync-skills") to
include this new key so the repo's Python autodoc command path is present and
uses node scripts/build-python-docs.mjs.
🪄 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
Run ID: 199c764f-cf03-4507-840d-0e502db6eb6c
📒 Files selected for processing (36)
.claude/skills/abstract-data-setup/SKILL.md.cursor/rules/abstract-data-setup.mdc.github/copilot-instructions.md.github/workflows/release-please.yml.release-please-manifest.jsonAGENTS.mdREADME.mdapps/playground/astro.config.mjsapps/playground/package.jsonapps/playground/scripts/build-python-docs.mjsapps/playground/scripts/pydoc-markdown.ymlapps/playground/scripts/python-autodoc.jsonapps/playground/src/content/docs/api/auditkit_batch.mdapps/playground/src/content/docs/api/auditkit_bootstrap.mdapps/playground/src/content/docs/api/auditkit_config.mdapps/playground/src/content/docs/api/auditkit_constants.mdapps/playground/src/content/docs/api/auditkit_core.mdapps/playground/src/content/docs/api/auditkit_services_authorization.mdapps/playground/src/content/docs/api/auditkit_transport_curl_impersonate.mdapps/playground/src/content/docs/api/example_module.mdapps/playground/src/content/docs/api/index.mdapps/playground/src/content/docs/recipes/python-autodoc.mdpackage.jsonpackages/create-docs/.gitignorepackages/create-docs/CHANGELOG.mdpackages/create-docs/README.mdpackages/create-docs/bin/cli.jspackages/create-docs/package.jsonpackages/starlight-theme/CHANGELOG.mdpackages/starlight-theme/package.jsonpackages/template/.claude/skills/abstract-data-setup/SKILL.mdpackages/template/.cursor/rules/abstract-data-setup.mdcpackages/template/.github/copilot-instructions.mdpackages/template/README.mdrelease-please-config.jsonscripts/compile-skill.mjs
| rev: 1.7.0 | ||
| hooks: | ||
| - id: interrogate | ||
| args: [--fail-under=80, -v, src/] |
There was a problem hiding this comment.
Avoid hardcoding src/ in the interrogate hook path
Line 189 hardcodes src/, but this workflow explicitly supports projects without a src layout. Use the detected package root/path from Phase 3 when composing hook args, otherwise coverage checks can miss real modules.
Suggested fix
- args: [--fail-under=80, -v, src/]
+ args: [--fail-under=80, -v, <detected_package_root_or_relative_package_path>]Based on learnings: Detect Python source projects by checking pyproject.toml, setup.py, requirements.txt, src/<pkg>/__init__.py, or <pkg>/__init__.py and identify package root from that result.
🤖 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 @.claude/skills/abstract-data-setup/SKILL.md at line 189, Replace the
hardcoded "src/" in the interrogate hook args (currently shown as args:
[--fail-under=80, -v, src/]) with the package root/path detected in Phase 3
(e.g., use the variable produced by your detection step such as package_root or
detected_source_path) so the hook runs against the real module layout; implement
detection by checking for pyproject.toml, setup.py, requirements.txt,
src/<pkg>/__init__.py, or <pkg>/__init__.py and inject that path into the
interrogate args, with a sensible fallback (current directory) if no package
root is found.
| publish-create-docs: | ||
| name: Publish @abstractdata/create-docs to npm | ||
| needs: release-please | ||
| if: ${{ needs.release-please.outputs.create_docs_released == 'true' }} | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: oven-sh/setup-bun@v2 | ||
| with: | ||
| bun-version: latest | ||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '20' | ||
| registry-url: 'https://registry.npmjs.org' | ||
| - run: bun install --frozen-lockfile | ||
| - name: Publish (prepack copies the template dir) | ||
| working-directory: packages/create-docs | ||
| run: npm publish --provenance --access public | ||
| env: | ||
| NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
publish-create-docs has no pre-publish quality gate.
publish-starlight-theme runs a typecheck before publishing; publish-create-docs goes straight from bun install to npm publish. Since packages/create-docs/package.json already defines "test": "node --test", it's low friction to add parity.
♻️ Proposed addition
- run: bun install --frozen-lockfile
+ - name: Test
+ working-directory: packages/create-docs
+ run: node --test
- name: Publish (prepack copies the template dir)
working-directory: packages/create-docs
run: npm publish --provenance --access public🤖 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 @.github/workflows/release-please.yml around lines 55 - 74,
publish-create-docs lacks a pre-publish quality gate; add a step before the
"Publish" step to run the package test/typecheck in the packages/create-docs
working directory (mirror what publish-starlight-theme does). Specifically,
insert a step named like "Run tests (quality gate)" with working-directory:
packages/create-docs and run either npm test or bun test (since package.json
defines "test": "node --test") so the job fails on test/typecheck failures
before npm publish; ensure NODE_AUTH_TOKEN remains only on the publish step.
| "build": "astro check && astro build", | ||
| "preview": "astro preview", | ||
| "typecheck": "astro check" | ||
| "typecheck": "astro check", | ||
| "docs:python": "node scripts/build-python-docs.mjs" | ||
| }, |
There was a problem hiding this comment.
Build pipeline can publish stale API docs.
docs:python was added, but build still skips it, so CI/deploy can ship outdated generated API pages unless someone remembers to run it manually.
Suggested patch
"scripts": {
"dev": "astro dev",
- "build": "astro check && astro build",
+ "build": "bun run docs:python && astro check && astro build",
"preview": "astro preview",
"typecheck": "astro check",
"docs:python": "node scripts/build-python-docs.mjs"
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "build": "astro check && astro build", | |
| "preview": "astro preview", | |
| "typecheck": "astro check" | |
| "typecheck": "astro check", | |
| "docs:python": "node scripts/build-python-docs.mjs" | |
| }, | |
| "scripts": { | |
| "dev": "astro dev", | |
| "build": "bun run docs:python && astro check && astro build", | |
| "preview": "astro preview", | |
| "typecheck": "astro check", | |
| "docs:python": "node scripts/build-python-docs.mjs" | |
| }, |
🤖 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 `@apps/playground/package.json` around lines 9 - 13, Build currently skips
generating Python docs, so update the npm scripts so the "build" script runs the
"docs:python" task before running "astro check" and "astro build"; modify the
"build" entry (and any CI/deploy steps that call it) to run "npm run
docs:python" (or the equivalent script invocation) first so generated API pages
are always up to date when executing the "build" command that currently
references "build" and the "docs:python" script names.
| try { | ||
| markdown = execSync( | ||
| `pydoc-markdown -I "${searchPath}" -m ${mod}`, | ||
| { encoding: 'utf8', stdio: ['ignore', 'pipe', 'inherit'] }, | ||
| ); | ||
| } catch { | ||
| log(`${c.red} ✗ ${mod}${c.reset}`); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Module name is unquoted in the shell command — path injection surface.
Line 81: `pydoc-markdown -I "${searchPath}" -m ${mod}` — ${mod} is not shell-quoted. While Python module names sourced from python-autodoc.json are unlikely to contain shell metacharacters in practice, a malicious or accidentally malformed config entry (e.g., "foo; rm -rf /") would be executed by the shell. Since cfg.modules is user-supplied, quoting ${mod} is the safe default.
🐛 Proposed fix
- `pydoc-markdown -I "${searchPath}" -m ${mod}`,
+ `pydoc-markdown -I "${searchPath}" -m "${mod}"`,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| markdown = execSync( | |
| `pydoc-markdown -I "${searchPath}" -m ${mod}`, | |
| { encoding: 'utf8', stdio: ['ignore', 'pipe', 'inherit'] }, | |
| ); | |
| } catch { | |
| log(`${c.red} ✗ ${mod}${c.reset}`); | |
| continue; | |
| } | |
| try { | |
| markdown = execSync( | |
| `pydoc-markdown -I "${searchPath}" -m "${mod}"`, | |
| { encoding: 'utf8', stdio: ['ignore', 'pipe', 'inherit'] }, | |
| ); | |
| } catch { | |
| log(`${c.red} ✗ ${mod}${c.reset}`); | |
| continue; | |
| } |
🤖 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 `@apps/playground/scripts/build-python-docs.mjs` around lines 79 - 87, The
command string passed to execSync embeds the user-controlled variable mod
unquoted, creating a shell injection risk; update the call to avoid shell
interpolation by using child_process.execFileSync (or execSync with shell
disabled and args array) so that pydoc-markdown is invoked with explicit args
instead of a single interpolated string, passing searchPath and mod as separate
arguments (reference the existing execSync call, the variables searchPath and
mod, and the pydoc-markdown invocation).
| const frontmatter = [ | ||
| '---', | ||
| `title: ${title}`, | ||
| `description: "${description}"`, | ||
| '---', | ||
| '', | ||
| ].join('\n'); | ||
|
|
||
| writeFileSync(outPath, frontmatter + body); |
There was a problem hiding this comment.
Unquoted title: in generated frontmatter will break Astro's YAML parser for any module with special characters in its title.
Line 129 writes title: ${title} without quoting. pydoc-markdown can generate titles containing colons (e.g., "auditkit.config: Application configuration") or other YAML special characters (#, [, {). An unquoted colon after a scalar breaks YAML parsing, which will cause bun run docs:python output to fail to load in Starlight.
The description: on line 130 is correctly double-quoted; title: should be too.
🐛 Proposed fix
const frontmatter = [
'---',
- `title: ${title}`,
+ `title: "${title.replace(/"/g, "'")}"`,
`description: "${description}"`,
'---',
'',
].join('\n');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const frontmatter = [ | |
| '---', | |
| `title: ${title}`, | |
| `description: "${description}"`, | |
| '---', | |
| '', | |
| ].join('\n'); | |
| writeFileSync(outPath, frontmatter + body); | |
| const frontmatter = [ | |
| '---', | |
| `title: "${title.replace(/"/g, "'")}"`, | |
| `description: "${description}"`, | |
| '---', | |
| '', | |
| ].join('\n'); | |
| writeFileSync(outPath, frontmatter + body); |
🤖 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 `@apps/playground/scripts/build-python-docs.mjs` around lines 127 - 135, The
frontmatter generation uses an unquoted title which breaks YAML when title
contains special characters; update the frontmatter assembly where the
frontmatter constant is built (the lines creating frontmatter and the template
entry `title: ${title}`) to emit a properly quoted and escaped title (e.g.,
`title: "${escapedTitle}"`) before calling writeFileSync(outPath, frontmatter +
body); ensure you create an escapedTitle by replacing any existing double quotes
and backslashes in the title string so the produced YAML is valid.
| ### `normalize` | ||
|
|
||
| ```py | ||
| def normalize(value: float, lo: float = 0.0, hi: float = 1.0) -> float | ||
| ``` | ||
|
|
||
| Clamp `value` to the inclusive `[lo, hi]` range. |
There was a problem hiding this comment.
normalize description says "Clamp" — conflicts with the function name.
Line 18 reads "Clamp value to the inclusive [lo, hi] range", but normalize conventionally means rescaling (mapping a value into a target range), not clamping (saturating at bounds). If the function actually clamps, rename it clamp; if it rescales/normalizes, update the description. As a demo page, this conflation could mislead users adopting this doc style.
🤖 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 `@apps/playground/src/content/docs/api/example_module.md` around lines 12 - 18,
The docs for the function `normalize` conflict with its name by saying "Clamp";
inspect the actual implementation of `normalize` and either (A) if it clamps,
rename the API and docs to `clamp` (update all occurrences of `normalize` in
docs and examples to `clamp`), or (B) if it rescales/normalizes, update the
description to explain the rescaling behavior (e.g., map input from its source
range into the inclusive [lo, hi] using the standard normalization/rescaling
formula) and adjust examples accordingly; reference the `normalize` symbol in
the codebase (or `clamp` if you choose to rename) to make the change consistent
across docs and examples.
| function copyRecursive(src, dst) { | ||
| mkdirSync(dst, { recursive: true }); | ||
| for (const entry of readdirSync(src, { withFileTypes: true })) { | ||
| if (entry.name === 'node_modules' || entry.name === 'dist' || entry.name === '.astro' || entry.name === 'bun.lock') continue; | ||
| const srcPath = join(src, entry.name); | ||
| const dstPath = join(dst, entry.name); | ||
| if (entry.isDirectory()) copyRecursive(srcPath, dstPath); | ||
| else copyFileSync(srcPath, dstPath); | ||
| } | ||
| } |
There was a problem hiding this comment.
copyRecursive doesn't exclude .git directories.
If the template directory contains a .git folder (which can happen in workspace dev scenarios), it would be copied into the new project. The subsequent git init at line 117 would then conflict with the copied .git, potentially leaving the project in a broken git state.
🐛 Proposed fix
- if (entry.name === 'node_modules' || entry.name === 'dist' || entry.name === '.astro' || entry.name === 'bun.lock') continue;
+ if (entry.name === 'node_modules' || entry.name === 'dist' || entry.name === '.astro' || entry.name === 'bun.lock' || entry.name === '.git') continue;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function copyRecursive(src, dst) { | |
| mkdirSync(dst, { recursive: true }); | |
| for (const entry of readdirSync(src, { withFileTypes: true })) { | |
| if (entry.name === 'node_modules' || entry.name === 'dist' || entry.name === '.astro' || entry.name === 'bun.lock') continue; | |
| const srcPath = join(src, entry.name); | |
| const dstPath = join(dst, entry.name); | |
| if (entry.isDirectory()) copyRecursive(srcPath, dstPath); | |
| else copyFileSync(srcPath, dstPath); | |
| } | |
| } | |
| function copyRecursive(src, dst) { | |
| mkdirSync(dst, { recursive: true }); | |
| for (const entry of readdirSync(src, { withFileTypes: true })) { | |
| if (entry.name === 'node_modules' || entry.name === 'dist' || entry.name === '.astro' || entry.name === 'bun.lock' || entry.name === '.git') continue; | |
| const srcPath = join(src, entry.name); | |
| const dstPath = join(dst, entry.name); | |
| if (entry.isDirectory()) copyRecursive(srcPath, dstPath); | |
| else copyFileSync(srcPath, dstPath); | |
| } | |
| } |
🤖 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 `@packages/create-docs/bin/cli.js` around lines 76 - 85, The copyRecursive
function currently copies .git directories into new projects; update
copyRecursive (the function named copyRecursive in
packages/create-docs/bin/cli.js) to skip any entry whose name is '.git' (add
'.git' to the existing exclusion check alongside 'node_modules', 'dist',
'.astro', 'bun.lock') so that directories named '.git' are not copied and
therefore won't conflict with the later git init step; ensure the check applies
before recursing (i.e., keep the exclusion logic at the start of the loop).
| // ─── Patch package.json ─────────────────────────────────────────────── | ||
| log(`${c.dim}→ wiring up${c.reset} package.json`); | ||
| const pkgPath = join(targetDir, 'package.json'); | ||
| const pkg = JSON.parse(readFileSync(pkgPath, 'utf8')); | ||
|
|
||
| pkg.name = projectName; | ||
| pkg.version = '0.0.1'; | ||
| pkg.private = true; | ||
| delete pkg.description; // user fills theirs | ||
|
|
||
| // Replace workspace:* with the published theme version range | ||
| if (pkg.dependencies?.['@abstractdata/starlight-theme']?.startsWith('workspace:')) { | ||
| pkg.dependencies['@abstractdata/starlight-theme'] = THEME_VERSION; | ||
| } | ||
|
|
||
| writeFileSync(pkgPath, JSON.stringify(pkg, null, 2) + '\n'); |
There was a problem hiding this comment.
Unhandled JSON parse error when reading the template's package.json.
Line 91 parses package.json without a try-catch. If the template's package.json is malformed (e.g., corrupted during packaging), this throws an uncaught SyntaxError with a Node.js stack trace instead of the user-friendly die() message.
🐛 Proposed fix
-const pkg = JSON.parse(readFileSync(pkgPath, 'utf8'));
+let pkg;
+try {
+ pkg = JSON.parse(readFileSync(pkgPath, 'utf8'));
+} catch {
+ die('Template package.json is malformed. Reinstall `@abstractdata/create-docs` or file an issue.');
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // ─── Patch package.json ─────────────────────────────────────────────── | |
| log(`${c.dim}→ wiring up${c.reset} package.json`); | |
| const pkgPath = join(targetDir, 'package.json'); | |
| const pkg = JSON.parse(readFileSync(pkgPath, 'utf8')); | |
| pkg.name = projectName; | |
| pkg.version = '0.0.1'; | |
| pkg.private = true; | |
| delete pkg.description; // user fills theirs | |
| // Replace workspace:* with the published theme version range | |
| if (pkg.dependencies?.['@abstractdata/starlight-theme']?.startsWith('workspace:')) { | |
| pkg.dependencies['@abstractdata/starlight-theme'] = THEME_VERSION; | |
| } | |
| writeFileSync(pkgPath, JSON.stringify(pkg, null, 2) + '\n'); | |
| // ─── Patch package.json ─────────────────────────────────────────────── | |
| log(`${c.dim}→ wiring up${c.reset} package.json`); | |
| const pkgPath = join(targetDir, 'package.json'); | |
| let pkg; | |
| try { | |
| pkg = JSON.parse(readFileSync(pkgPath, 'utf8')); | |
| } catch { | |
| die('Template package.json is malformed. Reinstall `@abstractdata/create-docs` or file an issue.'); | |
| } | |
| pkg.name = projectName; | |
| pkg.version = '0.0.1'; | |
| pkg.private = true; | |
| delete pkg.description; // user fills theirs | |
| // Replace workspace:* with the published theme version range | |
| if (pkg.dependencies?.['@abstractdata/starlight-theme']?.startsWith('workspace:')) { | |
| pkg.dependencies['@abstractdata/starlight-theme'] = THEME_VERSION; | |
| } | |
| writeFileSync(pkgPath, JSON.stringify(pkg, null, 2) + '\n'); |
🤖 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 `@packages/create-docs/bin/cli.js` around lines 88 - 103, The code reads and
JSON.parse's the template package.json into pkg (pkgPath, pkg) without error
handling; wrap the JSON.parse(readFileSync(pkgPath, 'utf8')) call in a
try/catch, and on catch call the existing die() helper with a clear,
user-friendly message that includes pkgPath and the parse error (or
error.message) so malformed package.json yields a controlled error instead of an
uncaught SyntaxError; ensure subsequent code (setting pkg.name/version/private,
dependency replacement, writeFileSync) only runs when parsing succeeds.
|
|
||
| ## What it does | ||
|
|
||
| 1. Copies the `@abstractdata/docs-template` files into a folder you name. |
There was a problem hiding this comment.
@abstractdata/docs-template doesn't exist — incorrect package name.
Line 23 says the CLI "copies the @abstractdata/docs-template files", but no such package exists in this monorepo or on npm. The template is bundled as a local template/ directory inside the published @abstractdata/create-docs package (copied from packages/template/ during prepack). Users searching for @abstractdata/docs-template will find nothing.
📝 Proposed fix
-1. Copies the `@abstractdata/docs-template` files into a folder you name.
+1. Copies the bundled template files into a folder you name.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 1. Copies the `@abstractdata/docs-template` files into a folder you name. | |
| 1. Copies the bundled template files into a folder you 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 `@packages/create-docs/README.md` at line 23, Replace the incorrect package
name `@abstractdata/docs-template` in the README with the actual template
location: explain that the CLI copies the local template bundled inside the
`@abstractdata/create-docs` package (the `template/` directory, copied from
`packages/template/` during prepack), e.g. change the sentence to say "Copies
the `template/` files bundled inside the `@abstractdata/create-docs` package" so
users aren't pointed to a non-existent npm package.
| function genericize(body) { | ||
| return body | ||
| .replace(/Use `AskUserQuestion`(\s+for every choice)?/g, | ||
| 'Ask the user via your interactive prompt mechanism$1') | ||
| .replace(/AskUserQuestion(:?)/g, 'an interactive prompt$1') | ||
| .replace(/the `AskUserQuestion` tool/g, 'your interactive prompt mechanism') | ||
| .replace(/Use `AskUserQuestion`/g, 'Ask the user'); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
genericize: line 110's Use \AskUserQuestion`` replace is already fully covered by line 106.
Line 106 uses Use \AskUserQuestion`(\s+for every choice)?with an optional group, so it already replaces the bareUse `AskUserQuestion`` form. Line 110 will never match any remaining text. The redundancy is harmless but worth removing for clarity.
♻️ Proposed cleanup
function genericize(body) {
return body
.replace(/Use `AskUserQuestion`(\s+for every choice)?/g,
'Ask the user via your interactive prompt mechanism$1')
.replace(/AskUserQuestion(:?)/g, 'an interactive prompt$1')
- .replace(/the `AskUserQuestion` tool/g, 'your interactive prompt mechanism')
- .replace(/Use `AskUserQuestion`/g, 'Ask the user');
+ .replace(/the `AskUserQuestion` tool/g, 'your interactive prompt mechanism');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function genericize(body) { | |
| return body | |
| .replace(/Use `AskUserQuestion`(\s+for every choice)?/g, | |
| 'Ask the user via your interactive prompt mechanism$1') | |
| .replace(/AskUserQuestion(:?)/g, 'an interactive prompt$1') | |
| .replace(/the `AskUserQuestion` tool/g, 'your interactive prompt mechanism') | |
| .replace(/Use `AskUserQuestion`/g, 'Ask the user'); | |
| } | |
| function genericize(body) { | |
| return body | |
| .replace(/Use `AskUserQuestion`(\s+for every choice)?/g, | |
| 'Ask the user via your interactive prompt mechanism$1') | |
| .replace(/AskUserQuestion(:?)/g, 'an interactive prompt$1') | |
| .replace(/the `AskUserQuestion` tool/g, 'your interactive prompt mechanism'); | |
| } |
🤖 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 `@scripts/compile-skill.mjs` around lines 104 - 111, Remove the redundant
replacement in function genericize: the final .replace(/Use `AskUserQuestion`/g,
'Ask the user') is unnecessary because the earlier .replace(/Use
`AskUserQuestion`(\s+for every choice)?/g, 'Ask the user via your interactive
prompt mechanism$1') already matches the bare "Use `AskUserQuestion`" form;
delete that last replace call to avoid duplication and clarify the intent of
genericize.
|
@copilot resolve the merge conflicts in this pull request |
|
@claude please fix the conflicts |
1 similar comment
|
@claude please fix the conflicts |
|
Claude finished @jreakin's task in 10m 33s —— View job Fixing Merge Conflicts
What I found: The PR branch has no common ancestor with The PR branch already has the correct, more complete content for all conflicting files — it's a superset of Why I couldn't complete this automatically: The core fix requires To resolve: run these commands locally git fetch origin
git checkout feat-add-create-docs-cli-package-ci
git pull --ff-only origin feat-add-create-docs-cli-package-ci # pick up Copilot's attempt if any
git merge origin/main --allow-unrelated-histories -X ours
git push
Alternative: GitHub's web conflict resolver at the bottom of the PR page can resolve individual file conflicts if you prefer a browser-based approach. What changes are needed per file (for manual reference):
|
Co-authored-by: jreakin <48845615+jreakin@users.noreply.github.com>
Merge conflicts resolved in c82f0f5. The following four conflicted files were reconciled:
New files from main ( |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| * npm create @abstractdata/docs@latest <project-name> | ||
| * pnpm create @abstractdata/docs <project-name> | ||
| */ | ||
| import { existsSync, mkdirSync, readdirSync, copyFileSync, readFileSync, writeFileSync, statSync } from 'node:fs'; |
| * pnpm create @abstractdata/docs <project-name> | ||
| */ | ||
| import { existsSync, mkdirSync, readdirSync, copyFileSync, readFileSync, writeFileSync, statSync } from 'node:fs'; | ||
| import { join, resolve, dirname, basename } from 'node:path'; |
Summary- Add
create-docsCLI, package, template copy, and CI publish workflow- Add Python Abstract autodoc setup documentation generationSummary by CodeRabbit
New Features
Theme
Documentation