feat(docs): add source-driven versioned API reference docs#17
Merged
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
WalkthroughBuild scripts now support per-tag, worktree-driven versioned API docs and emit version frontmatter; a new VersionPicker UI and theme apiBase wiring are added; Phase 11.7 documents an interactive versioning setup; recipes, playground docs, and scaffold/package metadata are updated. ChangesVersioned API Documentation Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 9
🤖 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 `@apps/playground/src/content/docs/recipes/versioned-docs.md`:
- Around line 10-21: Update the decision-tree fenced code block that starts with
"Are guides (concepts, how-tos) the same across versions?" to include a language
hint by changing the opening fence from ``` to ```text and ensure the closing
fence remains ```, so markdownlint MD040 is satisfied; locate that fenced block
in versioned-docs.md and make this single-line edit.
In `@packages/starlight-theme/src/components/VersionPicker.astro`:
- Around line 160-172: The change handler attached to select via
select.addEventListener currently unconditionally sets window.location.href to
target and doesn't implement the promised fallback to the version root when the
equivalent page 404s; update the handler (the function closing over
value/option/isDefault/versionSegment/target) to first perform a fetch (HEAD or
GET) to target, check the response status, and only navigate to target if the
response is OK (200); if the fetch returns 404 (or network error), set
window.location.href to the version root (compose base + apiBase +
versionSegment + '/'), preserving the existing trailing-slash normalization, and
ensure any fetch errors default to the version root as well.
- Around line 154-173: connectedCallback adds a new change listener on the
select every time it runs, causing duplicate handlers; fix by registering a
stable handler (e.g., store a bound function on the component instance like
this._onVersionChange) and use that same function when calling
select.addEventListener, and remove it in disconnectedCallback (or check/remove
existing listener before adding) so the handler attached in connectedCallback
and removed in disconnectedCallback keeps listeners from stacking; update
connectedCallback, implement disconnectedCallback, and reference the select
element and the handler name (e.g., this._onVersionChange / connectedCallback /
disconnectedCallback) when making the change.
In `@packages/template/.claude/skills/abstract-data-setup/SKILL.md`:
- Line 410: The release-tag selection currently uses a plain lexicographic git
tag listing; change the tag listing to explicitly sort by semantic version
descending (use git tag --list 'v*' with --sort=-v:refname) before taking the
top 5 candidates and setting the default; update the logic around "Surface up to
the 5 most recent tags" and the "Mark the most recent as `default: true`" step
so the candidate array is built from the sorted results and the first element is
treated as the default unless the user picks another tag.
- Around line 443-448: The example currently replaces starlight.components
outright; change the guidance to instruct merging SocialIcons into the existing
components map instead of assigning a new object—i.e., in astro.config.mjs use
the existing starlight.components and add or overwrite the SocialIcons key only.
Mention using a merge pattern (preserve existing keys) and recommend idempotency
checks before editing: ensure no duplicate sidebar entries, no duplicate
modules/entryPoints arrays, no repeated logo lines, and that abstractData()
plugin calls are not added twice; reference the starlight configuration, the
components object, and the SocialIcons override so maintainers know where to
apply the merge.
In `@packages/template/scripts/build-python-docs.mjs`:
- Around line 100-106: The current validation uses !versions.some((v) => v.tag)
which only ensures at least one entry has a tag; change this to require every
entry to have a tag by replacing that check with if (!versions.every((v) => v &&
v.tag)) die('Every entry in `versions` must have a `tag`.'); and likewise update
any duplicated checks in the other mirrored build-python-docs.mjs files; keep
the existing empty-array and default-version checks intact and run the new
.every validation before using versions[0].tag.
- Around line 128-129: The versioned build currently reuses an existing
directory (versionDir) which leaves stale markdown when modules/tags are
removed; change the logic around versionDir (computed from safeTag(version.tag)
and outputDir) to remove the existing directory first (e.g., rm -rf equivalent)
and then recreate it with mkdirSync(..., { recursive: true }) so regenerated
pages replace old files; apply the same deletion+recreate change to the other
mirrored build-python-docs.mjs copies in this PR.
In `@packages/template/scripts/build-ts-docs.mjs`:
- Around line 84-90: The validation currently uses versions.some(...) which only
ensures at least one item passes; change the checks to require every entry to be
valid: replace usages like !versions.some((v) => v.tag) with !versions.every((v)
=> v.tag && v.tag.length) (or equivalent) so the script rejects configs where
any versions[] item is missing/empty tag, and similarly ensure only one default
and that at least one default exists; update the same validation logic in the
other mirrored build-ts-docs.mjs copies; this prevents malformed items from
reaching safeTag(v.tag) or git worktree add later.
- Around line 113-114: The build leaves stale files because mkdirSync(outDir)
only ensures the directory exists; change the build step to remove any existing
output before recreating it by calling a recursive remove on outDir (e.g.,
rmSync/outDir removal with recursive:true and force:true) and then recreate it
with mkdirSync; apply this change around the code that computes outDir (using
safeTag(version.tag) / ROOT_OUTPUT) in build-ts-docs.mjs and mirror the same
removal+mkdir logic in the other copied build-ts-docs.mjs files in the PR so
each version (including the un-versioned default) starts with a clean directory.
🪄 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: a5a86cef-9bd5-45d0-a161-ceffb2443561
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
.claude/skills/abstract-data-setup/SKILL.md.cursor/rules/abstract-data-setup.mdc.github/copilot-instructions.mdapps/playground/astro.config.mjsapps/playground/scripts/build-python-docs.mjsapps/playground/scripts/build-ts-docs.mjsapps/playground/src/content/docs/recipes/versioned-docs.mdpackages/create-docs/CHANGELOG.mdpackages/create-docs/bin/cli.jspackages/create-docs/package.jsonpackages/starlight-theme/CHANGELOG.mdpackages/starlight-theme/package.jsonpackages/starlight-theme/scripts/build-python-docs.mjspackages/starlight-theme/scripts/build-ts-docs.mjspackages/starlight-theme/skills/claude/abstract-data-setup/SKILL.mdpackages/starlight-theme/skills/cursor/abstract-data-setup.mdcpackages/starlight-theme/skills/github/copilot-instructions.mdpackages/starlight-theme/src/components/VersionPicker.astropackages/template/.claude/skills/abstract-data-setup/SKILL.mdpackages/template/.cursor/rules/abstract-data-setup.mdcpackages/template/.github/copilot-instructions.mdpackages/template/scripts/build-python-docs.mjspackages/template/scripts/build-ts-docs.mjs
Comment on lines
+10
to
+21
| ``` | ||
| Are guides (concepts, how-tos) the same across versions? | ||
| ├── Yes → only the API reference changes between versions | ||
| │ → Source-driven (Pattern 1) — recommended for the theme | ||
| │ | ||
| └── No → guides differ by version too | ||
| ├── Maintain per-version branches already? | ||
| │ → Branch-per-version (Pattern 3) | ||
| │ | ||
| └── Want everything archived together? | ||
| → starlight-versions plugin (Pattern 2) | ||
| ``` |
There was a problem hiding this comment.
Add a language hint to the decision-tree fenced block.
This trips markdownlint MD040 in the new doc page.
Proposed fix
-```
+```text
Are guides (concepts, how-tos) the same across versions?
├── Yes → only the API reference changes between versions
│ → Source-driven (Pattern 1) — recommended for the theme
│
└── No → guides differ by version too
├── Maintain per-version branches already?
│ → Branch-per-version (Pattern 3)
│
└── Want everything archived together?
→ starlight-versions plugin (Pattern 2)</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
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/recipes/versioned-docs.md around lines 10 -
21, Update the decision-tree fenced code block that starts with "Are guides
(concepts, how-tos) the same across versions?" to include a language hint by
changing the opening fence from totext and ensure the closing fence
remains ```, so markdownlint MD040 is satisfied; locate that fenced block in
versioned-docs.md and make this single-line edit.
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- d98c2f50 -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+154
to
+173
| connectedCallback() { | ||
| const select = this.querySelector('select'); | ||
| if (!(select instanceof HTMLSelectElement)) return; | ||
| const apiBase = (this.dataset.apiBase || '/api').replace(/\/$/, ''); | ||
| const currentPage = this.dataset.currentPage || ''; | ||
| const base = this.dataset.base || ''; | ||
| select.addEventListener('change', (e) => { | ||
| const value = (e.target as HTMLSelectElement).value; | ||
| const option = (e.target as HTMLSelectElement).selectedOptions[0]; | ||
| const isDefault = option?.dataset.default === 'true'; | ||
| // Default version's pages live at the un-versioned URL. | ||
| const versionSegment = isDefault ? '' : `/${value}`; | ||
| const target = `${base}${apiBase}${versionSegment}/${currentPage}`.replace(/\/+$/, '/'); | ||
| // First try the equivalent page in the chosen version. If it 404s | ||
| // we want to fall back to that version's index — but we can't | ||
| // detect 404 from a navigation, so trust the build: if the page | ||
| // exists for one version it usually exists for adjacent ones. | ||
| window.location.href = target; | ||
| }); | ||
| } |
There was a problem hiding this comment.
Avoid duplicate change handlers across reconnects.
connectedCallback() can run more than once; each run adds another listener. In client-side navigation scenarios this can stack duplicate handlers.
Suggested fix
class StarlightVersionPicker extends HTMLElement {
+ `#bound` = false;
connectedCallback() {
const select = this.querySelector('select');
if (!(select instanceof HTMLSelectElement)) return;
+ if (this.#bound) return;
+ this.#bound = true;
const apiBase = (this.dataset.apiBase || '/api').replace(/\/$/, '');
const currentPage = this.dataset.currentPage || '';
const base = this.dataset.base || '';🤖 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/starlight-theme/src/components/VersionPicker.astro` around lines 154
- 173, connectedCallback adds a new change listener on the select every time it
runs, causing duplicate handlers; fix by registering a stable handler (e.g.,
store a bound function on the component instance like this._onVersionChange) and
use that same function when calling select.addEventListener, and remove it in
disconnectedCallback (or check/remove existing listener before adding) so the
handler attached in connectedCallback and removed in disconnectedCallback keeps
listeners from stacking; update connectedCallback, implement
disconnectedCallback, and reference the select element and the handler name
(e.g., this._onVersionChange / connectedCallback / disconnectedCallback) when
making the change.
Comment on lines
+160
to
+172
| select.addEventListener('change', (e) => { | ||
| const value = (e.target as HTMLSelectElement).value; | ||
| const option = (e.target as HTMLSelectElement).selectedOptions[0]; | ||
| const isDefault = option?.dataset.default === 'true'; | ||
| // Default version's pages live at the un-versioned URL. | ||
| const versionSegment = isDefault ? '' : `/${value}`; | ||
| const target = `${base}${apiBase}${versionSegment}/${currentPage}`.replace(/\/+$/, '/'); | ||
| // First try the equivalent page in the chosen version. If it 404s | ||
| // we want to fall back to that version's index — but we can't | ||
| // detect 404 from a navigation, so trust the build: if the page | ||
| // exists for one version it usually exists for adjacent ones. | ||
| window.location.href = target; | ||
| }); |
There was a problem hiding this comment.
Implement the advertised missing-page fallback during version switch.
The change handler always navigates to the equivalent page path. When that page does not exist in the target version, users hit a 404 instead of falling back to the version root as documented.
Suggested fix
- select.addEventListener('change', (e) => {
+ select.addEventListener('change', async (e) => {
const value = (e.target as HTMLSelectElement).value;
const option = (e.target as HTMLSelectElement).selectedOptions[0];
const isDefault = option?.dataset.default === 'true';
// Default version's pages live at the un-versioned URL.
const versionSegment = isDefault ? '' : `/${value}`;
const target = `${base}${apiBase}${versionSegment}/${currentPage}`.replace(/\/+$/, '/');
- // First try the equivalent page in the chosen version. If it 404s
- // we want to fall back to that version's index — but we can't
- // detect 404 from a navigation, so trust the build: if the page
- // exists for one version it usually exists for adjacent ones.
- window.location.href = target;
+ const root = `${base}${apiBase}${versionSegment}/`.replace(/\/+$/, '/');
+ try {
+ const probe = await fetch(target, { method: 'HEAD' });
+ window.location.href = probe.ok ? target : root;
+ } catch {
+ window.location.href = root;
+ }
});📝 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.
Suggested change
| select.addEventListener('change', (e) => { | |
| const value = (e.target as HTMLSelectElement).value; | |
| const option = (e.target as HTMLSelectElement).selectedOptions[0]; | |
| const isDefault = option?.dataset.default === 'true'; | |
| // Default version's pages live at the un-versioned URL. | |
| const versionSegment = isDefault ? '' : `/${value}`; | |
| const target = `${base}${apiBase}${versionSegment}/${currentPage}`.replace(/\/+$/, '/'); | |
| // First try the equivalent page in the chosen version. If it 404s | |
| // we want to fall back to that version's index — but we can't | |
| // detect 404 from a navigation, so trust the build: if the page | |
| // exists for one version it usually exists for adjacent ones. | |
| window.location.href = target; | |
| }); | |
| select.addEventListener('change', async (e) => { | |
| const value = (e.target as HTMLSelectElement).value; | |
| const option = (e.target as HTMLSelectElement).selectedOptions[0]; | |
| const isDefault = option?.dataset.default === 'true'; | |
| // Default version's pages live at the un-versioned URL. | |
| const versionSegment = isDefault ? '' : `/${value}`; | |
| const target = `${base}${apiBase}${versionSegment}/${currentPage}`.replace(/\/+$/, '/'); | |
| const root = `${base}${apiBase}${versionSegment}/`.replace(/\/+$/, '/'); | |
| try { | |
| const probe = await fetch(target, { method: 'HEAD' }); | |
| window.location.href = probe.ok ? target : root; | |
| } catch { | |
| window.location.href = root; | |
| } | |
| }); |
🤖 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/starlight-theme/src/components/VersionPicker.astro` around lines 160
- 172, The change handler attached to select via select.addEventListener
currently unconditionally sets window.location.href to target and doesn't
implement the promised fallback to the version root when the equivalent page
404s; update the handler (the function closing over
value/option/isDefault/versionSegment/target) to first perform a fetch (HEAD or
GET) to target, check the response status, and only navigate to target if the
response is OK (200); if the fetch returns 404 (or network error), set
window.location.href to the version root (compose base + apiBase +
versionSegment + '/'), preserving the existing trailing-slash normalization, and
ensure any fetch errors default to the version root as well.
|
|
||
| If the user picks option 1: | ||
|
|
||
| a. Surface up to the 5 most recent tags as candidates. Ask which to publish — let them deselect noisy point releases. Mark the most recent as `default: true` unless the user picks a different one (e.g. an LTS tag). |
There was a problem hiding this comment.
Sort the release tags before calling one “most recent.”
This phase tells the agent to surface the 5 most recent v* tags, but it never specifies a sort. Plain git tag --list 'v*' is lexicographic, so v0.10.0 can be treated as older than v0.9.0. Please spell out an explicit sort such as --sort=-v:refname before selecting candidates/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 `@packages/template/.claude/skills/abstract-data-setup/SKILL.md` at line 410,
The release-tag selection currently uses a plain lexicographic git tag listing;
change the tag listing to explicitly sort by semantic version descending (use
git tag --list 'v*' with --sort=-v:refname) before taking the top 5 candidates
and setting the default; update the logic around "Surface up to the 5 most
recent tags" and the "Mark the most recent as `default: true`" step so the
candidate array is built from the sorted results and the first element is
treated as the default unless the user picks another tag.
Comment on lines
+100
to
+106
| if (versions) { | ||
| if (versions.length === 0) die('`versions` is an empty array — set it to null/omit, or list at least one version.'); | ||
| if (!versions.some((v) => v.tag)) die('`versions[].tag` is required on every entry.'); | ||
| if (versions.filter((v) => v.default).length > 1) die('Only one `versions[].default: true` allowed.'); | ||
| if (!versions.some((v) => v.default)) { | ||
| log(`${c.gold}warn${c.reset} no version marked default; treating the first one (${versions[0].tag}) as default`); | ||
| versions[0].default = true; |
There was a problem hiding this comment.
Reject the config unless every version has a tag.
!versions.some((v) => v.tag) only proves that one entry is valid. A partially malformed versions array still gets through and fails later when the script tries to derive paths/worktrees from undefined. This check needs to validate every item. Please apply the same fix to the mirrored build-python-docs.mjs copies in this PR.
🤖 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/template/scripts/build-python-docs.mjs` around lines 100 - 106, The
current validation uses !versions.some((v) => v.tag) which only ensures at least
one entry has a tag; change this to require every entry to have a tag by
replacing that check with if (!versions.every((v) => v && v.tag)) die('Every
entry in `versions` must have a `tag`.'); and likewise update any duplicated
checks in the other mirrored build-python-docs.mjs files; keep the existing
empty-array and default-version checks intact and run the new .every validation
before using versions[0].tag.
Comment on lines
+128
to
+129
| const versionDir = version ? join(outputDir, safeTag(version.tag)) : outputDir; | ||
| mkdirSync(versionDir, { recursive: true }); |
There was a problem hiding this comment.
Clear versionDir before writing regenerated pages.
The versioned build now reuses existing directories, so removed modules and removed tags leave stale markdown behind. Those orphaned pages will still be picked up by Starlight’s autogenerated sidebar on later runs. Delete versionDir first, then recreate it. Please apply the same change to the mirrored build-python-docs.mjs copies in this PR.
Suggested fix
function buildOnce({ searchPath, version }) {
// version may be null (single-version mode) or { tag, label, default }
const versionDir = version ? join(outputDir, safeTag(version.tag)) : outputDir;
+ rmSync(versionDir, { recursive: true, force: true });
mkdirSync(versionDir, { recursive: true });📝 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.
Suggested change
| const versionDir = version ? join(outputDir, safeTag(version.tag)) : outputDir; | |
| mkdirSync(versionDir, { recursive: true }); | |
| const versionDir = version ? join(outputDir, safeTag(version.tag)) : outputDir; | |
| rmSync(versionDir, { recursive: true, force: true }); | |
| mkdirSync(versionDir, { recursive: true }); |
🤖 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/template/scripts/build-python-docs.mjs` around lines 128 - 129, The
versioned build currently reuses an existing directory (versionDir) which leaves
stale markdown when modules/tags are removed; change the logic around versionDir
(computed from safeTag(version.tag) and outputDir) to remove the existing
directory first (e.g., rm -rf equivalent) and then recreate it with
mkdirSync(..., { recursive: true }) so regenerated pages replace old files;
apply the same deletion+recreate change to the other mirrored
build-python-docs.mjs copies in this PR.
Comment on lines
+84
to
+90
| if (versions) { | ||
| if (versions.length === 0) die('`versions` is an empty array — set it to null/omit, or list at least one version.'); | ||
| if (!versions.some((v) => v.tag)) die('`versions[].tag` is required on every entry.'); | ||
| if (versions.filter((v) => v.default).length > 1) die('Only one `versions[].default: true` allowed.'); | ||
| if (!versions.some((v) => v.default)) { | ||
| log(`${c.gold}warn${c.reset} no version marked default; treating the first one (${versions[0].tag}) as default`); | ||
| versions[0].default = true; |
There was a problem hiding this comment.
Validate every versions[] item, not just one.
!versions.some((v) => v.tag) passes as soon as a single entry has a tag, so a malformed item still reaches safeTag(v.tag) / git worktree add later. This should reject the config unless every version has a non-empty tag. Please apply the same fix to the mirrored build-ts-docs.mjs copies in this PR.
🤖 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/template/scripts/build-ts-docs.mjs` around lines 84 - 90, The
validation currently uses versions.some(...) which only ensures at least one
item passes; change the checks to require every entry to be valid: replace
usages like !versions.some((v) => v.tag) with !versions.every((v) => v.tag &&
v.tag.length) (or equivalent) so the script rejects configs where any versions[]
item is missing/empty tag, and similarly ensure only one default and that at
least one default exists; update the same validation logic in the other mirrored
build-ts-docs.mjs copies; this prevents malformed items from reaching
safeTag(v.tag) or git worktree add later.
Comment on lines
+113
to
+114
| const outDir = version ? join(ROOT_OUTPUT, safeTag(version.tag)) : ROOT_OUTPUT; | ||
| mkdirSync(outDir, { recursive: true }); |
There was a problem hiding this comment.
Remove the previous build output before regenerating a version.
mkdirSync() alone leaves stale .md files behind. If a tag is deselected or an entry point stops emitting a page, the old files still sit under api/... and continue to appear in the autogenerated sidebar. Clear outDir before invoking TypeDoc for each build, including the un-versioned default alias. Please apply the same change to the mirrored build-ts-docs.mjs copies in this PR.
Suggested fix
function buildOnce({ entryPoints, tsconfig, version }) {
const outDir = version ? join(ROOT_OUTPUT, safeTag(version.tag)) : ROOT_OUTPUT;
+ rmSync(outDir, { recursive: true, force: true });
mkdirSync(outDir, { recursive: true });🤖 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/template/scripts/build-ts-docs.mjs` around lines 113 - 114, The
build leaves stale files because mkdirSync(outDir) only ensures the directory
exists; change the build step to remove any existing output before recreating it
by calling a recursive remove on outDir (e.g., rmSync/outDir removal with
recursive:true and force:true) and then recreate it with mkdirSync; apply this
change around the code that computes outDir (using safeTag(version.tag) /
ROOT_OUTPUT) in build-ts-docs.mjs and mirror the same removal+mkdir logic in the
other copied build-ts-docs.mjs files in the PR so each version (including the
un-versioned default) starts with a clean directory.
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/starlight-theme/scripts/build-ts-docs.mjs (1)
1-350: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftExtract
build-ts-docs.mjsinto a shared module to eliminate exact duplication across three locations.
packages/template/scripts/build-ts-docs.mjs,apps/playground/scripts/build-ts-docs.mjs, andpackages/starlight-theme/scripts/build-ts-docs.mjsare byte-identical. Each script that imports and invokes the build logic must be updated whenever the implementation changes, multiplying maintenance burden. Hoist the implementation into a shared module exported from@abstractdata/starlight-theme(or a small internal package) and have each script import and invokerunBuild().🤖 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/starlight-theme/scripts/build-ts-docs.mjs` around lines 1 - 350, The three identical scripts should be refactored into a shared module that exports a single entry function (e.g. runBuild) to centralize the logic currently implemented by safeTag, findGitRoot, buildOnce, cleanup and the top-level orchestration; create a new module in the shared package (e.g. `@abstractdata/starlight-theme`) that accepts configuration (project root, cfg path or parsed cfg, entryPoints, outputDir, versions, etc.), moves the TypeDoc invocation, post-processing, and worktree/version handling into that module, and exports runBuild(); then replace each script (packages/template/scripts/build-ts-docs.mjs, apps/playground/scripts/build-ts-docs.mjs, packages/starlight-theme/scripts/build-ts-docs.mjs) with a thin loader that imports runBuild from the shared module and calls it with the same parameters (preserving behavior of safeTag, buildOnce, findGitRoot, cleanup and logging), ensuring tests/CI still call the loader entry.
♻️ Duplicate comments (14)
packages/template/scripts/build-python-docs.mjs (1)
102-102: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winSame three issues as the other
build-python-docs.mjscopies.This file is byte-identical to
apps/playground/scripts/build-python-docs.mjsandpackages/starlight-theme/scripts/build-python-docs.mjs, so the same three fixes apply here:
- Line 102 — replace
versions.some((v) => v.tag)withversions.every((v) => v && v.tag)(past review comment).- Line 133-134 —
rmSync(versionDir, { recursive: true, force: true })beforemkdirSyncso dropped modules/tags don't leave orphans (past review comment).- Line 236 / Line 380 — guard
cfg.outputDirwith?? 'src/content/docs/api'; otherwise non-default version builds crash whenoutputDiris omitted from the config.See the proposed diffs on
apps/playground/scripts/build-python-docs.mjsfor details. Worth extracting into a single shared module given the byte-for-byte duplication.Also applies to: 133-134, 232-248
🤖 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/template/scripts/build-python-docs.mjs` at line 102, Replace the three problematic spots: (1) change the versions validation to use a safe check versions.every((v) => v && v.tag) instead of versions.some((v) => v.tag) to ensure every entry has a tag and avoid crashes on undefined entries (look for the versions variable/validation). (2) before calling mkdirSync for the per-version folder (versionDir), call rmSync(versionDir, { recursive: true, force: true }) to remove stale/orphaned files when tags/modules are dropped (look for versionDir, rmSync, mkdirSync usage around the version creation block). (3) guard access to cfg.outputDir by using the nullish fallback (cfg.outputDir ?? 'src/content/docs/api') wherever cfg.outputDir is read for output paths (targeting the uses around the referenced build steps), so non-default version configs that omit outputDir do not crash.packages/starlight-theme/scripts/build-python-docs.mjs (1)
102-102: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winThird copy of the same script — apply the same three fixes here.
Same content as the playground and template copies, so the same fixes are needed:
- Line 102 —
versions.every((v) => v && v.tag)(past review comment).- Line 133-134 — clear
versionDirbefore mkdir (past review comment).- Line 236 / Line 380 —
cfg.outputDir ?? 'src/content/docs/api'to avoid theTypeErrorwhenoutputDiris omitted.Maintaining three byte-identical copies of this 380-line script is going to keep generating mirrored review churn — worth promoting it to a shared module exposed from
@abstractdata/starlight-theme/scripts(or similar) and having the playground/template scripts thin-shim into it.Also applies to: 133-134, 232-248
🤖 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/starlight-theme/scripts/build-python-docs.mjs` at line 102, Replace the existence check on versions so it requires each entry and its tag (change the predicate from versions.some(...) to versions.every((v) => v && v.tag) or equivalent) in the block that validates versions, ensure the code that creates per-version directories clears the target versionDir before mkdir (add a rm/rmdir/empty-directory step for versionDir in the function that handles version directory creation/replication), and guard access to cfg.outputDir by using the nullish coalescing default cfg.outputDir ?? 'src/content/docs/api' wherever outputDir is used (replace direct uses of cfg.outputDir with that expression in the functions that write API docs).apps/playground/scripts/build-ts-docs.mjs (4)
119-122:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame stale-
outDirissue as inpackages/template/scripts/build-ts-docs.mjs.Mirror copy — apply the
rmSync(outDir, { recursive: true, force: true })call here too.🤖 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-ts-docs.mjs` around lines 119 - 122, The buildOnce function creates outDir but doesn't remove stale contents before writing; add a call to rmSync(outDir, { recursive: true, force: true }) immediately after computing outDir (before mkdirSync) to mirror packages/template/scripts/build-ts-docs.mjs so old files are removed; update the block inside buildOnce (referencing outDir and mkdirSync) to first rmSync(outDir, { recursive: true, force: true }) then mkdirSync(outDir, { recursive: true }).
86-86:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame
versions[].somevalidation gap as inpackages/template/scripts/build-ts-docs.mjs.This script is a mirror copy. Apply the
.everyvalidation fix raised on the template copy here as well.🤖 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-ts-docs.mjs` at line 86, The validation currently uses versions.some((v) => v.tag) which misses entries lacking tag; change it to require all entries by using versions.every((v) => v.tag) and keep the same die(...) call (i.e., replace the some check with an every check so the error " `versions[].tag` is required on every entry." is triggered when any entry is missing tag).
219-223:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSame default-alias link mismatch as in
packages/template/scripts/build-ts-docs.mjs.Mirror copy — point
latestPathat the un-versioned URL or drop the alias build.🤖 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-ts-docs.mjs` around lines 219 - 223, The default-alias (latestPath) currently points to a versioned URL; update the logic in the build that computes latestPath (using apiBase, sameRel, defaultV, safeTag) so that when defaultV is present it points to the un-versioned URL (e.g. `${apiBase}/${sameRel}/`) instead of `${apiBase}/${safeTag(defaultV.tag)}/${sameRel}/`, or remove producing the alias entirely; adjust the expression that sets latestPath in the code that uses cfg.outputDir, outDir and page.file to reflect this change.
181-191:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winSame YAML-escaping issue as in
packages/template/scripts/build-ts-docs.mjs.Mirror copy — apply the same
yamlEscape/ quoted-scalar fix here. Without it, a TypeDoc H1 likeClass: Fooproduces invalid frontmatter and breaks the build.🤖 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-ts-docs.mjs` around lines 181 - 191, The frontmatter assembly in build-ts-docs.mjs is injecting raw title/description/version strings into fmLines causing invalid YAML for values like "Class: Foo"; update the pushes to use the existing yamlEscape (or quoted-scalar) helper: replace `title`, `description`, `version.tag` and `version.label` uses in the fmLines.push/template literals with yamlEscape(...) (ensure description remains a quoted scalar if your helper requires it), i.e., call yamlEscape(title), yamlEscape(description), yamlEscape(version.tag) and yamlEscape(version.label) when building fmLines before joining into frontmatter.packages/starlight-theme/scripts/build-ts-docs.mjs (4)
119-122:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame stale-
outDirissue as in the template/playground copies.Mirror copy —
rmSync(outDir, { recursive: true, force: true })beforemkdirSynchere too.🤖 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/starlight-theme/scripts/build-ts-docs.mjs` around lines 119 - 122, The buildOnce function computes outDir (const outDir = version ? join(ROOT_OUTPUT, safeTag(version.tag)) : ROOT_OUTPUT) but doesn’t clean stale files; before calling mkdirSync(outDir, { recursive: true }) add a removal step rmSync(outDir, { recursive: true, force: true }) to ensure the directory is cleared first; update the buildOnce function to call rmSync(outDir, { recursive: true, force: true }) immediately before mkdirSync so outDir is recreated cleanly.
86-86:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame
versions[].somevalidation gap as in the template/playground copies.Mirror copy — apply the
.everyvalidation fix here too.🤖 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/starlight-theme/scripts/build-ts-docs.mjs` at line 86, The current validation uses versions.some((v) => v.tag) which only checks if any entry has a tag; change it to require every entry to have a tag by using the equivalent all-check (replace the versions.some((v) => v.tag) condition with a versions.every((v) => v.tag) check) so the die('`versions[].tag` is required on every entry.') branch triggers when any entry is missing a tag; update the line that calls die(...) accordingly where the versions validation occurs.
181-191:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winSame YAML-escaping issue as in the template/playground copies.
Mirror copy — apply the same
yamlEscape/ quoted-scalar fix here.🤖 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/starlight-theme/scripts/build-ts-docs.mjs` around lines 181 - 191, The frontmatter assembly in build-ts-docs.mjs currently inserts raw title/description/version strings into fmLines; apply the same yaml-escaping/quoted-scalar fix used elsewhere by passing title and description through the yamlEscape helper (or the project's quoted-scalar function) and use yamlEscape on version.tag and version.label before pushing them into fmLines so all frontmatter values are safely escaped/quoted; update the usages around fmLines.push for `title`, `description`, `version:`, `versionLabel:` to use the escaped values and keep the existing `versionDefault` logic unchanged.
219-223:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSame default-alias link mismatch as in the template/playground copies.
Mirror copy — point
latestPathat the un-versioned URL or drop the alias build.🤖 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/starlight-theme/scripts/build-ts-docs.mjs` around lines 219 - 223, The default-alias link currently points to a versioned URL; change how latestPath is computed inside build-ts-docs.mjs so that when defaultV is truthy latestPath uses the un-versioned path (combine apiBase and sameRel, e.g. `${apiBase}/${sameRel}/`) or make latestPath null to drop the alias build—update the logic around the latestPath declaration (referencing latestPath, apiBase, defaultV, safeTag, sameRel) to remove the safeTag/version segment so the alias targets the un-versioned URL.packages/template/scripts/build-ts-docs.mjs (2)
119-122:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear
outDirbefore regenerating; otherwise stale pages persist across builds.
mkdirSync(outDir, { recursive: true })is a no-op when the directory already exists, so files from a previous build remain when an entry point is removed, a tag is dropped fromversions[], or TypeDoc reorganizes its output. Those stale.mdfiles then get picked up by Starlight's autogenerated sidebar.Suggested fix
function buildOnce({ entryPoints, tsconfig, version }) { const outDir = version ? join(ROOT_OUTPUT, safeTag(version.tag)) : ROOT_OUTPUT; + rmSync(outDir, { recursive: true, force: true }); mkdirSync(outDir, { recursive: true });🤖 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/template/scripts/build-ts-docs.mjs` around lines 119 - 122, The buildOnce function currently creates outDir with mkdirSync(outDir, { recursive: true }) but does not clear it, leaving stale files; change buildOnce to remove or empty the existing outDir before recreating it (e.g., use fs.rmSync(outDir, { recursive: true, force: true }) or equivalent safe recursive delete), then recreate the directory with mkdirSync so TypeDoc output is always regenerated cleanly; update references in buildOnce and any callers that assume outDir contents persist.
86-86:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate every
versions[]entry, not just one.
!versions.some((v) => v.tag)only fails when all entries are missingtag. A single entry withtag: ""(or omitted) still slips through and lands insafeTag(v.tag)andgit worktree add ... "", which then fails far away from the source of the error. Reject the config unless every entry has a non-emptytag.Suggested fix
- if (!versions.some((v) => v.tag)) die('`versions[].tag` is required on every entry.'); + if (!versions.every((v) => typeof v.tag === 'string' && v.tag.length > 0)) { + die('`versions[].tag` is required on every entry.'); + }🤖 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/template/scripts/build-ts-docs.mjs` at line 86, The current check uses versions.some((v) => v.tag) which only fails if all entries lack tags; change it to assert every entry has a non-empty tag (e.g., use versions.every and ensure v.tag is truthy/non-empty or trimmed) so the script dies early with the same error message when any versions[] entry has a missing or empty tag; update the validation before safeTag(v.tag) and before invoking git worktree add to prevent downstream failures.packages/starlight-theme/src/components/VersionPicker.astro (2)
220-232:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing 404 fallback when the equivalent page does not exist in the target version.
The doc comment at lines 32–35 promises a fallback to the version's API root when the equivalent page is missing, but the change handler unconditionally navigates to
target. A symbol added/removed across versions will land users on a 404 instead. AHEADprobe before navigation closes that gap.Suggested fix
- select.addEventListener('change', (e) => { + select.addEventListener('change', async (e) => { const value = (e.target as HTMLSelectElement).value; const option = (e.target as HTMLSelectElement).selectedOptions[0]; const isDefault = option?.dataset.default === 'true'; const versionSegment = isDefault ? '' : `/${value}`; const target = `${base}${apiBase}${versionSegment}/${currentPage}`.replace(/\/+$/, '/'); - // First try the equivalent page in the chosen version. If it 404s - // we want to fall back to that version's index — but we can't - // detect 404 from a navigation, so trust the build: if the page - // exists for one version it usually exists for adjacent ones. - window.location.href = target; + const root = `${base}${apiBase}${versionSegment}/`.replace(/\/+$/, '/'); + try { + const probe = await fetch(target, { method: 'HEAD' }); + window.location.href = probe.ok ? target : root; + } catch { + window.location.href = root; + } });🤖 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/starlight-theme/src/components/VersionPicker.astro` around lines 220 - 232, The current change handler on select (the select.addEventListener('change', ...) block) navigates unconditionally to target and lacks the promised 404 fallback; update the handler to perform a HEAD probe (fetch(target, { method: 'HEAD' })) and only set window.location.href = target if the response.ok (status 2xx); otherwise build the version root (use the same base, apiBase and versionSegment logic but ending with '/') and navigate there; ensure you catch fetch errors and treat them as non-OK so the code falls back to the version index, and preserve the existing replace(/\/+$/, '/') trimming behavior when forming both target and fallback URLs.
213-237:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStacking
changelisteners on repeatedconnectedCallbackruns.In Starlight's client-side (View Transitions / Swup-style) navigation, the same custom-element instance can be reattached and
connectedCallbackre-runs each time. Without adisconnectedCallback(or a guard), each reconnection appends anotherchangelistener, so a single user interaction may trigger multiple navigations.Suggested fix
class StarlightVersionPicker extends HTMLElement { + `#onChange` = null; connectedCallback() { const select = this.querySelector('select'); if (!(select instanceof HTMLSelectElement)) return; + if (this.#onChange) return; const apiBase = (this.dataset.apiBase || '/api').replace(/\/$/, ''); const currentPage = this.dataset.currentPage || ''; const base = this.dataset.base || ''; - select.addEventListener('change', (e) => { /* … */ }); + this.#onChange = (e) => { /* … */ }; + select.addEventListener('change', this.#onChange); + } + disconnectedCallback() { + const select = this.querySelector('select'); + if (select && this.#onChange) select.removeEventListener('change', this.#onChange); + this.#onChange = null; } }🤖 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/starlight-theme/src/components/VersionPicker.astro` around lines 213 - 237, The connectedCallback on StarlightVersionPicker is adding a new 'change' listener every time the element is reattached; modify the class to avoid stacking listeners by storing the handler (e.g., this._onChange) as an instance property, add it in connectedCallback only if not already set, and remove it in disconnectedCallback using removeEventListener (or alternatively check for an existing listener before adding). Ensure you reference the same handler when calling addEventListener/removeEventListener on the select element so multiple navigations are not triggered on a single change.
🤖 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:
- Around line 393-417: The docs currently say to pick the “5 most recent tags”
using the ambiguous command `git -C <source-repo> tag --list 'v*'`; change the
wording and the workflow to use a deterministic sort (e.g., semver/version sort
or creator-date sort) so the candidate list and default selection are repeatable
— update the text that surfaces “up to the 5 most recent tags” and replace the
ambiguous git command with an explicit sort rule (mentioning the version-sort
and date-sort options) and state which sort is the default for semver projects
and which is the alternative for date-based policies.
In `@apps/playground/scripts/build-python-docs.mjs`:
- Around line 232-248: The code crashes when cfg.outputDir is undefined because
the "Older version" banner and the sidebar-wiring log call
cfg.outputDir.replace(...) directly; use the resolved local outputDir (the
variable defined around line 58 that defaults to 'src/content/docs/api') or
compute a safeRelativeOutput = (cfg.outputDir ??
outputDir).replace(/^src\/content\/docs\/?/, '').replace(/\/$/, '') and use that
in place of cfg.outputDir.replace(...) when building latestPath and in the
closing sidebar-wiring log (the block around lines 232–248 and again ~378–384);
apply the same fix to the other two copies of build-python-docs.mjs and check
build-ts-docs.mjs for identical patterns.
In `@apps/playground/scripts/python-autodoc.json`:
- Around line 1-10: Restore the repoUrl and repoBranch fields to the
python-autodoc.json config so the build scripts' conditional check (the touched
&& cfg.repoUrl branch used by build-python-docs.mjs and build-ts-docs.mjs) can
generate "View source on GitHub" links; add "repoUrl": "<actual repo HTTPS URL>"
and "repoBranch": "<branch name>" (or explicit null/empty documented in this
file if you intend to disable links) and update the JSON so cfg.repoUrl is
defined for playground versions, or alternatively add a clear comment in the
file documenting that GitHub source links are intentionally removed and that
build scripts will skip link generation when cfg.repoUrl is absent.
In `@apps/playground/src/content/docs/api/0-1-0/auditkit_bootstrap.md`:
- Line 13: The heading "load_default_registry" is using level 4 (####) which
creates a jump in the Markdown hierarchy; change the heading markup for
"load_default_registry" from "#### load_default_registry" to "##
load_default_registry" (or the appropriate level consistent with surrounding
headings) to restore proper heading order and satisfy the linter.
In `@apps/playground/src/content/docs/api/0-1-0/auditkit_config.md`:
- Line 40: The subheading "http_transport_backend_order_tuple" under the
"AuditConfig Objects" section is using a level-4 heading (####) which breaks the
document hierarchy; change that heading (and the other similar headings in the
same section, e.g., the one noted at the other occurrence) from "####" to "###"
so all object subheadings consistently use level-3 under the main "AuditConfig
Objects" header.
In `@apps/playground/src/content/docs/api/synthetic-v0-0-1/auditkit_bootstrap.md`:
- Line 16: The heading "load_default_registry" is using #### which skips heading
levels; change the heading markup for "load_default_registry" from "####
load_default_registry" to "## load_default_registry" so the document's first
explicit heading is an h2 (matching the frontmatter title) and avoids
markdownlint MD001; update the heading token in the auditkit_bootstrap.md file
where the "load_default_registry" heading appears.
In `@apps/playground/src/content/docs/api/synthetic-v0-0-5/auditkit_config.md`:
- Around line 43-57: The markdown uses fourth-level headings for property docs
which breaks nesting rules; change the headings for
http_transport_backend_order_tuple and assume_cms_tuple from "####" to "###" so
they match the surrounding section hierarchy and fix MD001; update the two
headers that currently read "#### http_transport_backend_order_tuple" and "####
assume_cms_tuple" to "### http_transport_backend_order_tuple" and "###
assume_cms_tuple" respectively (these correspond to the property docs for the
http_transport_backend_order_tuple and assume_cms_tuple properties).
In `@packages/starlight-theme/src/components/VersionPicker.astro`:
- Around line 86-109: Auto-discovery currently only sets a version's label on
the first seen page and never updates it if later pages provide a non-empty
versionLabel; update the loop that processes entries (the block using seen,
seen.set, existing, data.versionLabel, and existing.default) so that when an
existing Version is found you still assign its label if the existing.label
equals its tag (or is empty) and data.versionLabel is non-empty, while
preserving the current logic that flips existing.default to true if
data.versionDefault is present; keep propVersions/getCollection logic unchanged
but normalize label resolution to prefer the first non-empty versionLabel
encountered across pages.
- Line 67: The VersionPicker component defaults apiBase to '/api' (const {
versions: propVersions, apiBase = '/api' } = Astro.props), which can mismatch
the autodoc build output (e.g., '/api/ts') and silently break URL detection when
consumers render <VersionPicker> directly; update VersionPicker.astro to use a
default that aligns with the theme/build convention (e.g., match the autodoc
outputDir) or surface a clear runtime warning and require an explicit apiBase
prop, and ensure SocialIcons or the theme's virtual:abstractdata/config still
injects apiBase when used via the recommended pattern.
In `@packages/template/scripts/build-ts-docs.mjs`:
- Line 268: The footer’s "View on GitHub" link uses the const branch = version ?
version.tag : (cfg.repoBranch ?? 'main'); which mixes tags and branch names and
can be confusing to future maintainers; add a short inline comment next to the
branch definition explaining that for versioned builds we intentionally use
version.tag (the worktree/tag supplied to git worktree add) while for
unversioned builds we use cfg.repoBranch (or 'main'), and note that GitHub
accepts tags or branches in the /tree/ URL so this preservation of the raw tag
is intentional and should not be normalized.
- Line 295: The current signal handler only listens for SIGINT and so SIGTERM,
SIGHUP, and unhandled exceptions/rejections can bypass cleanup and leak the
mkdtempSync worktree; replace or augment the single process.on('SIGINT', ...)
usage by adding handlers for SIGTERM and SIGHUP that call cleanup() and exit
with appropriate codes, and add uncaughtException and unhandledRejection
listeners that invoke cleanup() before rethrowing or calling process.exit(1);
reference the existing cleanup() function, the current process.on('SIGINT', ...)
handler, and the code area around mkdtempSync and the surrounding try/finally to
insert these additional handlers so cleanup always runs on termination or fatal
errors.
- Line 219: The computed latest link includes safeTag(defaultV.tag) which points
to the versioned mirror instead of the un-versioned default alias; update the
logic that builds latestPath (and its two mirrored copies) to omit the safeTag
segment when generating links for the default-alias build (i.e. derive apiBase
from cfg.outputDir as you already do but detect ROOT_OUTPUT/default alias runs
and build latestPath as `${apiBase}/${sameRel}/` rather than
`${apiBase}/${safeTag(defaultV.tag)}/${sameRel}/`), or alternatively skip
emitting the default-alias build—adjust the code paths around apiBase,
latestPath and the defaultV.tag usage to ensure default pages point to the
un-versioned root URL.
- Around line 181-191: The YAML frontmatter building code pushes raw title and
description into fmLines without escaping/quoting which breaks YAML for values
containing ":" or backslashes; update the fmLines construction (the lines that
push `title: ${title}` and `description: "${description}"`) to produce safely
quoted and escaped scalars — either escape backslashes and double-quotes in
title/description before injecting or, better, serialize the frontmatter object
via a YAML library (or use single-quoted scalars and escape single quotes by
doubling) so `title` and `description` are always valid YAML; apply the same fix
to the other mirrored build-ts-docs.mjs copies that use the same fmLines logic.
- Around line 137-139: The execSync invocation in build-ts-docs.mjs that
constructs a shell command with args (the execSync call using `npx typedoc
${args.map(...)}` and PROJECT_ROOT/stdio) is vulnerable to shell injection;
replace it with execFileSync so the executable is the first argument ('npx') and
the remaining items are passed as an array (['typedoc', ...args]) with the same
options (cwd: PROJECT_ROOT, stdio: 'inherit') to avoid shell parsing; make the
analogous change in build-python-docs.mjs where pydoc-markdown is invoked via a
shell string (replace the string-constructed call with execFileSync using the
command and an args array that includes '-I', searchPath and '-m', mod, and
preserve cwd and stdio).
---
Outside diff comments:
In `@packages/starlight-theme/scripts/build-ts-docs.mjs`:
- Around line 1-350: The three identical scripts should be refactored into a
shared module that exports a single entry function (e.g. runBuild) to centralize
the logic currently implemented by safeTag, findGitRoot, buildOnce, cleanup and
the top-level orchestration; create a new module in the shared package (e.g.
`@abstractdata/starlight-theme`) that accepts configuration (project root, cfg
path or parsed cfg, entryPoints, outputDir, versions, etc.), moves the TypeDoc
invocation, post-processing, and worktree/version handling into that module, and
exports runBuild(); then replace each script
(packages/template/scripts/build-ts-docs.mjs,
apps/playground/scripts/build-ts-docs.mjs,
packages/starlight-theme/scripts/build-ts-docs.mjs) with a thin loader that
imports runBuild from the shared module and calls it with the same parameters
(preserving behavior of safeTag, buildOnce, findGitRoot, cleanup and logging),
ensuring tests/CI still call the loader entry.
---
Duplicate comments:
In `@apps/playground/scripts/build-ts-docs.mjs`:
- Around line 119-122: The buildOnce function creates outDir but doesn't remove
stale contents before writing; add a call to rmSync(outDir, { recursive: true,
force: true }) immediately after computing outDir (before mkdirSync) to mirror
packages/template/scripts/build-ts-docs.mjs so old files are removed; update the
block inside buildOnce (referencing outDir and mkdirSync) to first
rmSync(outDir, { recursive: true, force: true }) then mkdirSync(outDir, {
recursive: true }).
- Line 86: The validation currently uses versions.some((v) => v.tag) which
misses entries lacking tag; change it to require all entries by using
versions.every((v) => v.tag) and keep the same die(...) call (i.e., replace the
some check with an every check so the error " `versions[].tag` is required on
every entry." is triggered when any entry is missing tag).
- Around line 219-223: The default-alias (latestPath) currently points to a
versioned URL; update the logic in the build that computes latestPath (using
apiBase, sameRel, defaultV, safeTag) so that when defaultV is present it points
to the un-versioned URL (e.g. `${apiBase}/${sameRel}/`) instead of
`${apiBase}/${safeTag(defaultV.tag)}/${sameRel}/`, or remove producing the alias
entirely; adjust the expression that sets latestPath in the code that uses
cfg.outputDir, outDir and page.file to reflect this change.
- Around line 181-191: The frontmatter assembly in build-ts-docs.mjs is
injecting raw title/description/version strings into fmLines causing invalid
YAML for values like "Class: Foo"; update the pushes to use the existing
yamlEscape (or quoted-scalar) helper: replace `title`, `description`,
`version.tag` and `version.label` uses in the fmLines.push/template literals
with yamlEscape(...) (ensure description remains a quoted scalar if your helper
requires it), i.e., call yamlEscape(title), yamlEscape(description),
yamlEscape(version.tag) and yamlEscape(version.label) when building fmLines
before joining into frontmatter.
In `@packages/starlight-theme/scripts/build-python-docs.mjs`:
- Line 102: Replace the existence check on versions so it requires each entry
and its tag (change the predicate from versions.some(...) to versions.every((v)
=> v && v.tag) or equivalent) in the block that validates versions, ensure the
code that creates per-version directories clears the target versionDir before
mkdir (add a rm/rmdir/empty-directory step for versionDir in the function that
handles version directory creation/replication), and guard access to
cfg.outputDir by using the nullish coalescing default cfg.outputDir ??
'src/content/docs/api' wherever outputDir is used (replace direct uses of
cfg.outputDir with that expression in the functions that write API docs).
In `@packages/starlight-theme/scripts/build-ts-docs.mjs`:
- Around line 119-122: The buildOnce function computes outDir (const outDir =
version ? join(ROOT_OUTPUT, safeTag(version.tag)) : ROOT_OUTPUT) but doesn’t
clean stale files; before calling mkdirSync(outDir, { recursive: true }) add a
removal step rmSync(outDir, { recursive: true, force: true }) to ensure the
directory is cleared first; update the buildOnce function to call rmSync(outDir,
{ recursive: true, force: true }) immediately before mkdirSync so outDir is
recreated cleanly.
- Line 86: The current validation uses versions.some((v) => v.tag) which only
checks if any entry has a tag; change it to require every entry to have a tag by
using the equivalent all-check (replace the versions.some((v) => v.tag)
condition with a versions.every((v) => v.tag) check) so the
die('`versions[].tag` is required on every entry.') branch triggers when any
entry is missing a tag; update the line that calls die(...) accordingly where
the versions validation occurs.
- Around line 181-191: The frontmatter assembly in build-ts-docs.mjs currently
inserts raw title/description/version strings into fmLines; apply the same
yaml-escaping/quoted-scalar fix used elsewhere by passing title and description
through the yamlEscape helper (or the project's quoted-scalar function) and use
yamlEscape on version.tag and version.label before pushing them into fmLines so
all frontmatter values are safely escaped/quoted; update the usages around
fmLines.push for `title`, `description`, `version:`, `versionLabel:` to use the
escaped values and keep the existing `versionDefault` logic unchanged.
- Around line 219-223: The default-alias link currently points to a versioned
URL; change how latestPath is computed inside build-ts-docs.mjs so that when
defaultV is truthy latestPath uses the un-versioned path (combine apiBase and
sameRel, e.g. `${apiBase}/${sameRel}/`) or make latestPath null to drop the
alias build—update the logic around the latestPath declaration (referencing
latestPath, apiBase, defaultV, safeTag, sameRel) to remove the safeTag/version
segment so the alias targets the un-versioned URL.
In `@packages/starlight-theme/src/components/VersionPicker.astro`:
- Around line 220-232: The current change handler on select (the
select.addEventListener('change', ...) block) navigates unconditionally to
target and lacks the promised 404 fallback; update the handler to perform a HEAD
probe (fetch(target, { method: 'HEAD' })) and only set window.location.href =
target if the response.ok (status 2xx); otherwise build the version root (use
the same base, apiBase and versionSegment logic but ending with '/') and
navigate there; ensure you catch fetch errors and treat them as non-OK so the
code falls back to the version index, and preserve the existing replace(/\/+$/,
'/') trimming behavior when forming both target and fallback URLs.
- Around line 213-237: The connectedCallback on StarlightVersionPicker is adding
a new 'change' listener every time the element is reattached; modify the class
to avoid stacking listeners by storing the handler (e.g., this._onChange) as an
instance property, add it in connectedCallback only if not already set, and
remove it in disconnectedCallback using removeEventListener (or alternatively
check for an existing listener before adding). Ensure you reference the same
handler when calling addEventListener/removeEventListener on the select element
so multiple navigations are not triggered on a single change.
In `@packages/template/scripts/build-python-docs.mjs`:
- Line 102: Replace the three problematic spots: (1) change the versions
validation to use a safe check versions.every((v) => v && v.tag) instead of
versions.some((v) => v.tag) to ensure every entry has a tag and avoid crashes on
undefined entries (look for the versions variable/validation). (2) before
calling mkdirSync for the per-version folder (versionDir), call
rmSync(versionDir, { recursive: true, force: true }) to remove stale/orphaned
files when tags/modules are dropped (look for versionDir, rmSync, mkdirSync
usage around the version creation block). (3) guard access to cfg.outputDir by
using the nullish fallback (cfg.outputDir ?? 'src/content/docs/api') wherever
cfg.outputDir is read for output paths (targeting the uses around the referenced
build steps), so non-default version configs that omit outputDir do not crash.
In `@packages/template/scripts/build-ts-docs.mjs`:
- Around line 119-122: The buildOnce function currently creates outDir with
mkdirSync(outDir, { recursive: true }) but does not clear it, leaving stale
files; change buildOnce to remove or empty the existing outDir before recreating
it (e.g., use fs.rmSync(outDir, { recursive: true, force: true }) or equivalent
safe recursive delete), then recreate the directory with mkdirSync so TypeDoc
output is always regenerated cleanly; update references in buildOnce and any
callers that assume outDir contents persist.
- Line 86: The current check uses versions.some((v) => v.tag) which only fails
if all entries lack tags; change it to assert every entry has a non-empty tag
(e.g., use versions.every and ensure v.tag is truthy/non-empty or trimmed) so
the script dies early with the same error message when any versions[] entry has
a missing or empty tag; update the validation before safeTag(v.tag) and before
invoking git worktree add to prevent downstream failures.
🪄 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: 6096246a-785c-409f-a71f-7b4017f2e9f7
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (41)
.claude/skills/abstract-data-setup/SKILL.md.cursor/rules/abstract-data-setup.mdc.github/copilot-instructions.mdapps/playground/package.jsonapps/playground/scripts/build-python-docs.mjsapps/playground/scripts/build-ts-docs.mjsapps/playground/scripts/python-autodoc.jsonapps/playground/src/content.config.tsapps/playground/src/content/docs/api/0-1-0/auditkit_bootstrap.mdapps/playground/src/content/docs/api/0-1-0/auditkit_config.mdapps/playground/src/content/docs/api/0-1-0/auditkit_constants.mdapps/playground/src/content/docs/api/auditkit.mdapps/playground/src/content/docs/api/auditkit_batch.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/synthetic-v0-0-1/auditkit_bootstrap.mdapps/playground/src/content/docs/api/synthetic-v0-0-1/auditkit_config.mdapps/playground/src/content/docs/api/synthetic-v0-0-1/auditkit_constants.mdapps/playground/src/content/docs/api/synthetic-v0-0-5/auditkit_bootstrap.mdapps/playground/src/content/docs/api/synthetic-v0-0-5/auditkit_config.mdapps/playground/src/content/docs/api/synthetic-v0-0-5/auditkit_constants.mdapps/playground/src/content/docs/recipes/versioned-docs.mdpackages/create-docs/bin/cli.jspackages/starlight-theme/CHANGELOG.mdpackages/starlight-theme/package.jsonpackages/starlight-theme/scripts/build-python-docs.mjspackages/starlight-theme/scripts/build-ts-docs.mjspackages/starlight-theme/skills/claude/abstract-data-setup/SKILL.mdpackages/starlight-theme/skills/cursor/abstract-data-setup.mdcpackages/starlight-theme/skills/github/copilot-instructions.mdpackages/starlight-theme/src/components/SocialIcons.astropackages/starlight-theme/src/components/VersionPicker.astropackages/starlight-theme/src/index.tspackages/template/.claude/skills/abstract-data-setup/SKILL.mdpackages/template/.cursor/rules/abstract-data-setup.mdcpackages/template/.github/copilot-instructions.mdpackages/template/package.jsonpackages/template/scripts/build-python-docs.mjspackages/template/scripts/build-ts-docs.mjspackages/template/src/content.config.ts
💤 Files with no reviewable changes (5)
- apps/playground/src/content/docs/api/auditkit_core.md
- apps/playground/src/content/docs/api/auditkit_batch.md
- apps/playground/src/content/docs/api/auditkit_services_authorization.md
- apps/playground/src/content/docs/api/auditkit.md
- apps/playground/src/content/docs/api/auditkit_transport_curl_impersonate.md
Comment on lines
+393
to
+417
| Run `git -C <source-repo> tag --list 'v*' | wc -l` (or equivalent) on the **source** repo (not the docs project). If the result is < 2, skip this phase silently. | ||
|
|
||
| Otherwise, surface the choice: | ||
|
|
||
| > "I see your source has N tagged releases. Versioned API reference is supported four ways. Which do you want?" | ||
|
|
||
| Multi-choice prompt: | ||
|
|
||
| 1. **Source-driven (Recommended for API-only versioning).** Adds a `versions` array to `python-autodoc.json` / `ts-autodoc.json`. Each rebuild checks out the source repo at each tag (via `git worktree add`), regenerates the API reference per tag into `<outputDir>/<safeTag>/`, and aliases the default version at the un-versioned URL. Cheap, composable, no branches to maintain. The bundled `<VersionPicker>` component renders a topbar dropdown. | ||
| 2. **`starlight-versions` plugin** — opinionated, archives the entire site (guides + API). Pick this only if guides drift between versions too. Pre-1.0; expect rough edges. | ||
| 3. **Branch-per-version** — each major version is a git branch deployed to a subdomain, the main branch's host (Vercel/CF) rewrites `/v2/*` → that subdomain. Best when teams already maintain per-version branches. | ||
| 4. **Single version (no versioning).** Default if the user is unsure. Easy to add versioning later. | ||
|
|
||
| Default the recommendation to **option 1** when the project has Python or TypeScript autodoc wired (Phases 4/5 ran). Default to **option 2** when the docs project has substantial hand-written guides and the user expects them to differ per version. | ||
|
|
||
| If the user picks option 1: | ||
|
|
||
| a. Surface up to the 5 most recent tags as candidates. Ask which to publish — let them deselect noisy point releases. Mark the most recent as `default: true` unless the user picks a different one (e.g. an LTS tag). | ||
|
|
||
| b. Write the `versions` array into the appropriate autodoc JSON config. Example shape: | ||
|
|
||
| ```jsonc | ||
| { | ||
| "searchPath": "../../auditkit/src", | ||
| "modules": [...], |
There was a problem hiding this comment.
Define deterministic tag sorting before selecting the “5 most recent” versions.
Line 393/410 describe “most recent tags” but don’t specify sort semantics. git tag --list 'v*' is not a reliable “most recent” order, so this can pick the wrong default release.
Use an explicit sort rule (version or creator date) in the workflow text, e.g. --sort=-version:refname for semver-style tags.
#!/bin/bash
set -euo pipefail
SOURCE_REPO="${1:-../source-repo}"
echo "Raw listing (current ambiguous behavior):"
git -C "$SOURCE_REPO" tag --list 'v*' | head -n 10
echo
echo "Version-sorted candidates (recommended for semver tags):"
git -C "$SOURCE_REPO" tag --sort=-version:refname --list 'v*' | head -n 5
echo
echo "Date-sorted candidates (alternative):"
git -C "$SOURCE_REPO" for-each-ref \
--sort=-creatordate \
--format='%(refname:short)' refs/tags/v* | head -n 5Expected result: the selected 5 tags are deterministic and match the intended “recent/default” policy.
🧰 Tools
🪛 LanguageTool
[grammar] ~401-~401: Ensure spelling is correct
Context: ...d <VersionPicker> component renders a topbar dropdown. 2. **starlight-versions plu...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 around lines 393 - 417, The docs
currently say to pick the “5 most recent tags” using the ambiguous command `git
-C <source-repo> tag --list 'v*'`; change the wording and the workflow to use a
deterministic sort (e.g., semver/version sort or creator-date sort) so the
candidate list and default selection are repeatable — update the text that
surfaces “up to the 5 most recent tags” and replace the ambiguous git command
with an explicit sort rule (mentioning the version-sort and date-sort options)
and state which sort is the default for semver projects and which is the
alternative for date-based policies.
Comment on lines
+232
to
+248
| if (version && !version.default) { | ||
| const defaultVersion = (cfg.versions ?? []).find((v) => v.default); | ||
| const latestLabel = defaultVersion ? (defaultVersion.label ?? defaultVersion.tag) : 'latest'; | ||
| const latestPath = defaultVersion | ||
| ? `/${cfg.outputDir.replace(/^src\/content\/docs\/?/, '').replace(/\/$/, '')}/${safeTag(defaultVersion.tag)}/${page.safeName}/` | ||
| : null; | ||
| const link = latestPath ? `[${latestLabel} →](${latestPath})` : latestLabel; | ||
| const stale = [ | ||
| '', | ||
| `:::caution[Older version]`, | ||
| `You're viewing **${version.label ?? version.tag}**. Latest is ${link}.`, | ||
| ':::', | ||
| '', | ||
| ].join('\n'); | ||
| newBody = stale + newBody; | ||
| touched = true; | ||
| } |
There was a problem hiding this comment.
cfg.outputDir.replace(...) crashes when outputDir is omitted from the config.
The local outputDir constant on line 58 falls back to 'src/content/docs/api', but cfg.outputDir itself stays undefined if the user didn't set it. The "Older version" banner (line 236) and the closing sidebar-wiring log (line 380) call .replace(...) directly on cfg.outputDir. As soon as versions is configured without an explicit outputDir, building any non-default version throws TypeError: Cannot read properties of undefined (reading 'replace') and aborts the run. Easy to miss because the playground config sets outputDir explicitly.
Mirror this fix in the other two build-python-docs.mjs copies (and check build-ts-docs.mjs for the same pattern).
🛡️ Proposed fix — capture the resolved relative path once
const ORIGINAL_SEARCH_PATH = resolve(PROJECT_ROOT, cfg.searchPath);
-const outputDir = resolve(PROJECT_ROOT, cfg.outputDir ?? 'src/content/docs/api');
+const outputDirRel = cfg.outputDir ?? 'src/content/docs/api';
+const outputDir = resolve(PROJECT_ROOT, outputDirRel);
@@
- const latestPath = defaultVersion
- ? `/${cfg.outputDir.replace(/^src\/content\/docs\/?/, '').replace(/\/$/, '')}/${safeTag(defaultVersion.tag)}/${page.safeName}/`
- : null;
+ const latestPath = defaultVersion
+ ? `/${outputDirRel.replace(/^src\/content\/docs\/?/, '').replace(/\/$/, '')}/${safeTag(defaultVersion.tag)}/${page.safeName}/`
+ : null;
@@
-log(`${c.dim} { label: 'API Reference', autogenerate: { directory: '${cfg.outputDir.replace(/^src\/content\/docs\/?/, '')}' } }${c.reset}`);
+log(`${c.dim} { label: 'API Reference', autogenerate: { directory: '${outputDirRel.replace(/^src\/content\/docs\/?/, '')}' } }${c.reset}`);Also applies to: 378-384
🤖 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 232 - 248, The
code crashes when cfg.outputDir is undefined because the "Older version" banner
and the sidebar-wiring log call cfg.outputDir.replace(...) directly; use the
resolved local outputDir (the variable defined around line 58 that defaults to
'src/content/docs/api') or compute a safeRelativeOutput = (cfg.outputDir ??
outputDir).replace(/^src\/content\/docs\/?/, '').replace(/\/$/, '') and use that
in place of cfg.outputDir.replace(...) when building latestPath and in the
closing sidebar-wiring log (the block around lines 232–248 and again ~378–384);
apply the same fix to the other two copies of build-python-docs.mjs and check
build-ts-docs.mjs for identical patterns.
Comment on lines
1
to
10
| { | ||
| "$schema": "Configuration for scripts/build-python-docs.mjs. Adjust to point at your Python source.", | ||
| "searchPath": "../../../website-auditkit/src", | ||
| "modules": [ | ||
| "auditkit", | ||
| "auditkit.config", | ||
| "auditkit.bootstrap", | ||
| "auditkit.batch", | ||
| "auditkit.constants", | ||
| "auditkit.core", | ||
| "auditkit.transport.curl_impersonate", | ||
| "auditkit.services.authorization" | ||
| ], | ||
| "modules": ["auditkit.config", "auditkit.bootstrap", "auditkit.constants"], | ||
| "outputDir": "src/content/docs/api", | ||
| "repoUrl": "", | ||
| "repoBranch": "main" | ||
| "versions": [ | ||
| { "tag": "v0.1.0", "label": "0.1 (latest)", "default": true }, | ||
| { "tag": "synthetic-v0.0.5", "label": "0.0.5" }, | ||
| { "tag": "synthetic-v0.0.1", "label": "0.0.1 (legacy)" } | ||
| ] | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the build script still references repoUrl or repoBranch
fd "build-python-docs|build-ts-docs" --type f --exec grep -n "repoUrl\|repoBranch" {}Repository: Abstract-Data/abstract-data-doc-theme
Length of output: 1869
Add repoUrl and repoBranch back to the config or document their removal.
The build scripts reference these fields to generate "View source on GitHub" links (lines 267–269, 291–293 in build-python-docs.mjs and build-ts-docs.mjs). Since the code conditionally checks if (touched && cfg.repoUrl), removing these fields silently breaks source-link generation without a build error. Either restore them or document that GitHub links won't be available for playground versions.
🤖 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/python-autodoc.json` around lines 1 - 10, Restore the
repoUrl and repoBranch fields to the python-autodoc.json config so the build
scripts' conditional check (the touched && cfg.repoUrl branch used by
build-python-docs.mjs and build-ts-docs.mjs) can generate "View source on
GitHub" links; add "repoUrl": "<actual repo HTTPS URL>" and "repoBranch":
"<branch name>" (or explicit null/empty documented in this file if you intend to
disable links) and update the JSON so cfg.repoUrl is defined for playground
versions, or alternatively add a clear comment in the file documenting that
GitHub source links are intentionally removed and that build scripts will skip
link generation when cfg.repoUrl is absent.
| Assemble a module registry with all built-in scan modules registered. | ||
|
|
||
|
|
||
| #### load\_default\_registry |
There was a problem hiding this comment.
Fix heading level jump to satisfy Markdown structure/lint.
#### load_default_registry skips heading levels on this page. Use ## here to keep heading hierarchy valid.
Proposed fix
-#### load\_default\_registry
+## load\_default\_registry📝 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.
Suggested change
| #### load\_default\_registry | |
| ## load_default_registry |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
🤖 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/0-1-0/auditkit_bootstrap.md` at line 13,
The heading "load_default_registry" is using level 4 (####) which creates a jump
in the Markdown hierarchy; change the heading markup for "load_default_registry"
from "#### load_default_registry" to "## load_default_registry" (or the
appropriate level consistent with surrounding headings) to restore proper
heading order and satisfy the linter.
| AuditKit runtime configuration, loaded from environment variables. | ||
|
|
||
|
|
||
| #### http\_transport\_backend\_order\_tuple |
There was a problem hiding this comment.
Normalize subheading depth under AuditConfig Objects.
These headings jump from ## to ####. Change them to ### to preserve valid hierarchy.
Proposed fix
-#### http\_transport\_backend\_order\_tuple
+### http\_transport\_backend\_order\_tuple
...
-#### assume\_cms\_tuple
+### assume\_cms\_tupleAlso applies to: 50-50
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 40-40: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🤖 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/0-1-0/auditkit_config.md` at line 40,
The subheading "http_transport_backend_order_tuple" under the "AuditConfig
Objects" section is using a level-4 heading (####) which breaks the document
hierarchy; change that heading (and the other similar headings in the same
section, e.g., the one noted at the other occurrence) from "####" to "###" so
all object subheadings consistently use level-3 under the main "AuditConfig
Objects" header.
Comment on lines
+137
to
+139
| try { | ||
| execSync(`npx typedoc ${args.map((a) => `"${a}"`).join(' ')}`, | ||
| { cwd: PROJECT_ROOT, stdio: 'inherit' }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Sanity-check that no caller relies on the current shell-string form.
rg -nP -C2 'execSync\(`npx typedoc' --type=js --type=mjsRepository: Abstract-Data/abstract-data-doc-theme
Length of output: 109
🏁 Script executed:
#!/bin/bash
# Read the file to verify the code at lines 137-139
if [ -f "packages/template/scripts/build-ts-docs.mjs" ]; then
echo "=== File exists, reading lines 130-145 for context ==="
sed -n '130,145p' "packages/template/scripts/build-ts-docs.mjs"
else
echo "File not found at expected path"
# Try to locate it
find . -name "build-ts-docs.mjs" -type f
fiRepository: Abstract-Data/abstract-data-doc-theme
Length of output: 646
🏁 Script executed:
#!/bin/bash
# Search for execSync usage with npx - try without the mjs type specification
rg -n 'execSync.*npx\s+typedoc' --type-list | head -5 || true
rg -n 'execSync' packages/template/scripts/ || trueRepository: Abstract-Data/abstract-data-doc-theme
Length of output: 2023
🏁 Script executed:
#!/bin/bash
# Read more context around the vulnerable code
sed -n '120,150p' "packages/template/scripts/build-ts-docs.mjs"Repository: Abstract-Data/abstract-data-doc-theme
Length of output: 1172
🏁 Script executed:
#!/bin/bash
# Check where entryPoints and tsconfig come from
rg -n 'entryPoints|tsconfig' "packages/template/scripts/build-ts-docs.mjs" -B2 -A2 | head -60Repository: Abstract-Data/abstract-data-doc-theme
Length of output: 2641
🏁 Script executed:
#!/bin/bash
# Check if build-python-docs.mjs has similar vulnerable pattern
grep -n 'execSync.*pydoc' packages/template/scripts/build-python-docs.mjs -A2 -B2Repository: Abstract-Data/abstract-data-doc-theme
Length of output: 278
🏁 Script executed:
#!/bin/bash
# Verify the full context of line 152 in build-python-docs.mjs
sed -n '140,160p' packages/template/scripts/build-python-docs.mjsRepository: Abstract-Data/abstract-data-doc-theme
Length of output: 732
Replace execSync with execFileSync to avoid shell injection risks with user-controlled paths.
The manual shell quoting in execSync(npx typedoc ${args.map((a) => "${a}").join(' ')}, ...) does not escape quotes or backslashes within arguments. Since entryPoints and tsconfig are user-controlled paths (potentially from mkdtempSync worktrees), special characters or spaces could break the command. Windows shell handling of double-quotes is also notoriously fragile.
Replace with execFileSync('npx', ['typedoc', ...args], { cwd: PROJECT_ROOT, stdio: 'inherit' }) to bypass the shell entirely and pass arguments safely as an array.
A similar pattern exists in build-python-docs.mjs at line 152 with pydoc-markdown -I "${searchPath}" -m ${mod} that has the same vulnerability.
🤖 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/template/scripts/build-ts-docs.mjs` around lines 137 - 139, The
execSync invocation in build-ts-docs.mjs that constructs a shell command with
args (the execSync call using `npx typedoc ${args.map(...)}` and
PROJECT_ROOT/stdio) is vulnerable to shell injection; replace it with
execFileSync so the executable is the first argument ('npx') and the remaining
items are passed as an array (['typedoc', ...args]) with the same options (cwd:
PROJECT_ROOT, stdio: 'inherit') to avoid shell parsing; make the analogous
change in build-python-docs.mjs where pydoc-markdown is invoked via a shell
string (replace the string-constructed call with execFileSync using the command
and an args array that includes '-I', searchPath and '-m', mod, and preserve cwd
and stdio).
Comment on lines
+181
to
+191
| const fmLines = ['---', `title: ${title}`, `description: "${description}"`]; | ||
| if (version) { | ||
| // Emit version metadata. `versionDefault: true` lets the bundled | ||
| // <VersionPicker> auto-discover which version to pre-select without | ||
| // duplicating the canonical list outside the autodoc JSON config. | ||
| fmLines.push(`version: "${version.tag}"`); | ||
| if (version.label) fmLines.push(`versionLabel: "${version.label}"`); | ||
| if (version.default) fmLines.push(`versionDefault: true`); | ||
| } | ||
| fmLines.push('---', ''); | ||
| const frontmatter = fmLines.join('\n'); |
There was a problem hiding this comment.
Title/description aren't escaped before being injected into YAML frontmatter.
title: ${title} and description: "${description}" are written verbatim. TypeDoc's markdown plugin commonly produces H1s like Class: Foo or Type Alias: Bar<T>; an unquoted value containing : is invalid YAML and Astro/Starlight will fail to parse the frontmatter. Similarly, a description that lost its " chars to the replace(/"/g, "'") pass can still contain backslashes (e.g. escaped Markdown \_) that break the double-quoted YAML scalar.
Always quote the scalar and escape " / \ (or use a YAML library / single-quoted scalars with '' escaping).
Suggested fix
- const fmLines = ['---', `title: ${title}`, `description: "${description}"`];
+ const yamlEscape = (s) => s.replace(/\\/g, '\\\\').replace(/"/g, '\\"');
+ const fmLines = [
+ '---',
+ `title: "${yamlEscape(title)}"`,
+ `description: "${yamlEscape(description)}"`,
+ ];
if (version) {
- fmLines.push(`version: "${version.tag}"`);
- if (version.label) fmLines.push(`versionLabel: "${version.label}"`);
+ fmLines.push(`version: "${yamlEscape(version.tag)}"`);
+ if (version.label) fmLines.push(`versionLabel: "${yamlEscape(version.label)}"`);
if (version.default) fmLines.push(`versionDefault: true`);
}Apply the same change to the mirrored copies in apps/playground/scripts/build-ts-docs.mjs and packages/starlight-theme/scripts/build-ts-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.
Suggested change
| const fmLines = ['---', `title: ${title}`, `description: "${description}"`]; | |
| if (version) { | |
| // Emit version metadata. `versionDefault: true` lets the bundled | |
| // <VersionPicker> auto-discover which version to pre-select without | |
| // duplicating the canonical list outside the autodoc JSON config. | |
| fmLines.push(`version: "${version.tag}"`); | |
| if (version.label) fmLines.push(`versionLabel: "${version.label}"`); | |
| if (version.default) fmLines.push(`versionDefault: true`); | |
| } | |
| fmLines.push('---', ''); | |
| const frontmatter = fmLines.join('\n'); | |
| const yamlEscape = (s) => s.replace(/\\/g, '\\\\').replace(/"/g, '\\"'); | |
| const fmLines = [ | |
| '---', | |
| `title: "${yamlEscape(title)}"`, | |
| `description: "${yamlEscape(description)}"`, | |
| ]; | |
| if (version) { | |
| // Emit version metadata. `versionDefault: true` lets the bundled | |
| // <VersionPicker> auto-discover which version to pre-select without | |
| // duplicating the canonical list outside the autodoc JSON config. | |
| fmLines.push(`version: "${yamlEscape(version.tag)}"`); | |
| if (version.label) fmLines.push(`versionLabel: "${yamlEscape(version.label)}"`); | |
| if (version.default) fmLines.push(`versionDefault: true`); | |
| } | |
| fmLines.push('---', ''); | |
| const frontmatter = fmLines.join('\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/template/scripts/build-ts-docs.mjs` around lines 181 - 191, The YAML
frontmatter building code pushes raw title and description into fmLines without
escaping/quoting which breaks YAML for values containing ":" or backslashes;
update the fmLines construction (the lines that push `title: ${title}` and
`description: "${description}"`) to produce safely quoted and escaped scalars —
either escape backslashes and double-quotes in title/description before
injecting or, better, serialize the frontmatter object via a YAML library (or
use single-quoted scalars and escape single quotes by doubling) so `title` and
`description` are always valid YAML; apply the same fix to the other mirrored
build-ts-docs.mjs copies that use the same fmLines logic.
| if (version && !version.default) { | ||
| const defaultV = (cfg.versions ?? []).find((v) => v.default); | ||
| const latestLabel = defaultV ? (defaultV.label ?? defaultV.tag) : 'latest'; | ||
| const apiBase = `/${cfg.outputDir.replace(/^src\/content\/docs\/?/, '').replace(/\/$/, '')}`; |
There was a problem hiding this comment.
Latest-version link uses cfg.outputDir, but pages live at the un-versioned root for the default build.
The banner on a non-default version computes latestPath as ${apiBase}/${safeTag(defaultV.tag)}/${sameRel}/. However, the script also runs a "default alias" build that writes the default version's pages to the un-versioned root (ROOT_OUTPUT, no safeTag segment). So the banner points at /api/ts/0-3-0/foo/, but the canonical default URL is /api/ts/foo/. When a user clicks "latest", they leave the alias and end up on the versioned mirror — losing the un-versioned URL benefit.
Either drop the safeTag(defaultV.tag) segment from latestPath, or skip the default-alias build entirely. Same change needed in the two mirrored copies.
Suggested fix
- const latestPath = defaultV
- ? `${apiBase}/${safeTag(defaultV.tag)}/${sameRel}/`
- : null;
+ // Default version is also aliased at the un-versioned URL.
+ const latestPath = defaultV ? `${apiBase}/${sameRel}/` : null;🤖 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/template/scripts/build-ts-docs.mjs` at line 219, The computed latest
link includes safeTag(defaultV.tag) which points to the versioned mirror instead
of the un-versioned default alias; update the logic that builds latestPath (and
its two mirrored copies) to omit the safeTag segment when generating links for
the default-alias build (i.e. derive apiBase from cfg.outputDir as you already
do but detect ROOT_OUTPUT/default alias runs and build latestPath as
`${apiBase}/${sameRel}/` rather than
`${apiBase}/${safeTag(defaultV.tag)}/${sameRel}/`), or alternatively skip
emitting the default-alias build—adjust the code paths around apiBase,
latestPath and the defaultV.tag usage to ensure default pages point to the
un-versioned root URL.
| log(`${c.gold} ⚠${c.reset} thin-page banner on ${relative(outputDir, page.file)}`); | ||
|
|
||
| if (touched && cfg.repoUrl) { | ||
| const branch = version ? version.tag : (cfg.repoBranch ?? 'main'); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
${branch} in "View on GitHub" footer uses the raw tag, but cfg.repoBranch is a branch name.
For unversioned builds the footer points at ${repo}/tree/${cfg.repoBranch ?? 'main'}. For versioned builds it points at ${repo}/tree/${version.tag}. That's fine if tags exist in the source repo with the same name (v0.3.0 etc.), but tree/<tag> and tree/<branch> are interchangeable in GitHub's URL, so this works as long as the tag is exactly what was passed to git worktree add. Worth a comment so future readers don't normalize this away.
🤖 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/template/scripts/build-ts-docs.mjs` at line 268, The footer’s "View
on GitHub" link uses the const branch = version ? version.tag : (cfg.repoBranch
?? 'main'); which mixes tags and branch names and can be confusing to future
maintainers; add a short inline comment next to the branch definition explaining
that for versioned builds we intentionally use version.tag (the worktree/tag
supplied to git worktree add) while for unversioned builds we use cfg.repoBranch
(or 'main'), and note that GitHub accepts tags or branches in the /tree/ URL so
this preservation of the raw tag is intentional and should not be normalized.
| } | ||
| } | ||
| process.on('exit', cleanup); | ||
| process.on('SIGINT', () => { cleanup(); process.exit(130); }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
SIGTERM and uncaught exceptions skip cleanup.
process.on('exit', cleanup) runs on normal termination, but exit does not fire for SIGTERM (common in CI), SIGHUP, or unhandled exceptions/rejections. The try/finally covers the synchronous path, yet a fatal signal between mkdtempSync and the finally leaks the worktree. Hooking SIGTERM (and optionally uncaughtException/unhandledRejection) keeps the source repo clean.
🤖 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/template/scripts/build-ts-docs.mjs` at line 295, The current signal
handler only listens for SIGINT and so SIGTERM, SIGHUP, and unhandled
exceptions/rejections can bypass cleanup and leak the mkdtempSync worktree;
replace or augment the single process.on('SIGINT', ...) usage by adding handlers
for SIGTERM and SIGHUP that call cleanup() and exit with appropriate codes, and
add uncaughtException and unhandledRejection listeners that invoke cleanup()
before rethrowing or calling process.exit(1); reference the existing cleanup()
function, the current process.on('SIGINT', ...) handler, and the code area
around mkdtempSync and the surrounding try/finally to insert these additional
handlers so cleanup always runs on termination or fatal errors.
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 by CodeRabbit
New Features
Documentation
Chores
Build