Repository navigation
[Website] Patch sync-docs.js in place, add tests, fix edit links (#1308) - #1553
Open
andrewmusselman wants to merge 7 commits into
Open
andrewmusselman wants to merge 7 commits into
andrewmusselman wants to merge 7 commits into
Conversation
Make website/scripts/sync-docs.js importable without side effects: main() now only runs when the file is executed directly, and the markdown transforms are exported. The per-file work is split into a pure processMarkdown(content, filePath) function so it can be tested without touching the file system; processMarkdownFile() is now a thin read/transform/write wrapper. Add scripts/sync-docs.test.js using Node's built-in node:test runner (no new dependency) and an `npm test` script. These first tests pin the *current* behaviour (frontmatter split, Jekyll field removal, .md links left intact per #1305, directory sync and clean) so the following commits can change the internals safely. Also add the ASF license header that the script was missing. No change to generated output: `node scripts/sync-docs.js` produces a byte-identical website/docs and website/blog tree before and after. Part of #1308 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since #1305 removed the `.md` stripping, transformLinks() matched inline [text](url) links with a regex and returned them unchanged in every branch: it was an identity function that only looked like it did something. Keeping it invites someone to "fix" a link problem in the regex again, which is what caused the 404s in #1301. Remove it and document in processMarkdown() that the markdown body is copied byte for byte, because Docusaurus resolves relative `./x.md` links to version-aware URLs itself. A test pins pass-through for inline, autolink, reference-style, HTML, JSX and fenced-code links. The issue also floated unified + remark-parse for link transforms; with nothing left to transform there is nothing to parse, so no new dependency is added. If a real link rewrite is ever needed, an AST pass (or a Docusaurus remark plugin) is the right tool, not a regex. No change to generated output. Part of #1308 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The hand-rolled frontmatter parser split each line on the first ':' and the generator only quoted values containing ':' or '#'. That misparses YAML lists in block form, nested maps, folded/multi-line strings, CRLF files and empty blocks, and can write YAML that Docusaurus then reads differently from the source file. Use the same parser Docusaurus uses: @11ty/gray-matter (the maintained gray-matter fork that @docusaurus/utils 3.10 depends on; the original `gray-matter` package is NOT in our tree and still pulls js-yaml 3) on top of js-yaml 4. Both were already installed transitively; they are now explicit dependencies at ranges that resolve to the versions already locked, so `npm ci` installs no new packages. Write frontmatter back with js-yaml's dump, with options chosen to match how our docs are written today: no line folding, double quotes, inline lists, and date-only values kept as `2024-03-04` rather than expanded to an ISO timestamp. A YAML syntax error now fails the sync with the offending file name. The 22-line package-lock.json diff is from `npm install`: two lines for the new root dependencies, plus 20 `"dev": true` flags on the optional @typescript/* native binaries. Those 20 are npm normalising a lock that was already out of date (running `npm install --package-lock-only` on the untouched main branch produces the same 20 lines); they are not caused by this change. Verified: `node scripts/sync-docs.js` output is byte-identical to the previous script for all 102 synced docs and blog files. The new tests fail against the old parser (6 failures) and pass with this one. Part of #1308 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When a doc has no `title`, sync-docs derived one from the file name with s/[_-]/ /g plus \b\w capitalisation, so `qdp-api.md` became "Qdp Api" and `pqc.md` became "Pqc". Split the fallback into a pure titleFromFilename() that maps whole words through a small table of canonical spellings (QDP, API, GPU, CUDA, ROCm, QML, PQC, FAQ, PyTorch, NumPy, LLMs, ...) and drops a leading number prefix (`001-`) the same way Docusaurus does for doc IDs. transformFrontmatter() also no longer second-guesses the author: - an explicit title is never replaced, including `title: ""` (valid in Docusaurus) and non-string values; - if the body starts with a `# Heading`, no title is injected, since Docusaurus already uses that heading and it is a better title than a guess from the file name. Every page under /docs sets `title` today, so this is a latent bug and the synced output is byte-identical; the tests cover the fallback path directly. Part of #1308 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Docusaurus derives the edit link from the directory it reads, joined onto `editUrl`. Because docs and blog posts are read from copies made by sync-docs.js, every generated link was wrong: - current docs: tree/main/docs/docs/<page>.md (doubled "docs/") - released docs: tree/main/docs/versioned_docs/... (should be website/versioned_docs/...) - blog posts: tree/main/website-new/blog/... (pre-migration path) All 226 unique edit URLs in a local build pointed at paths that do not exist in the repository. Use the function form of `editUrl` for docs and blog so the links target what contributors should edit: /docs/<page> for the current version and /docs/blog/<post> for the blog (the sync's source of truth), and website/versioned_docs/version-X/<page> for released snapshots. After this change all 226 URLs resolve to tracked files (checked with `git ls-files --error-unmatch`). The rendered HTML is otherwise unchanged. This is a direct cost of syncing into a copy, so it belongs with the sync-docs work; it would disappear under the Docusaurus-native layout discussed in the ADR in this series. Part of #1308 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add an `npm test` step to the Website Build Check workflow so the node:test suite for scripts/sync-docs.js runs on every PR that touches website/ or docs/. It takes well under a second and needs no extra setup beyond the existing `npm ci`. In website/README.md, describe what the sync does and does not rewrite (frontmatter only; links are passed through and should keep their `.md` extension), mention `npm test`, and correct the Blog Posts section, which said posts live in website/blog/ (a gitignored copy) rather than /docs/blog/. Part of #1308 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add a *proposed* ADR laying out the second option from #1308 (Docusaurus-native `docs.path` / `blog.path` instead of syncing /docs into website/docs) next to the patched-in-place sync, so maintainers can decide with the facts in one place. It records the #882 decision the sync came from and leaves the Decision section open. The findings come from throwaway local builds against 2d94f8f (not committed): - docs.path -> ../docs builds and renders the same pages; - excluding docs/api also hides versioned_docs/*/api, so the generator config would need to move (e.g. docs/_api); - blog.path nested inside docs.path breaks all 60 blog routes, so a native layout needs the blog moved out of /docs; - `docs:version` copies docs.path wholesale and ignores `exclude`, which matters for snapshots and for generated API pages. The file is placed in dev/adr/ rather than docs/adr/ so that the proposal is not published on the website. ADR-001 was removed from docs/adr/ in #1384, and its copies survive only in versioned_docs, so reviewers may prefer a different home; this is easy to move. Part of #1308 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Purpose of PR
Implements option 1 from #1308 (patch the sync in place), since the sync exists by design (#882). The script gets a real YAML parser, tests, and acronym-aware fallback titles; the no-op link regex goes away; and "Edit this page" links, which all pointed at non-existent paths, now point at the real source files. Option 2 (Docusaurus-native loading) is written up as a proposed ADR so maintainers can decide.
Related Issues or PRs
Part of #1308
Changes Made
website/scripts/sync-docs.js: importable without side effects; pureprocessMarkdown(); ASF header. Newsync-docs.test.js(node:test) andnpm test.transformLinks()(identity function since docs: fix stale links on the How to Contribute pages (#1301) #1305); bodies are copied byte for byte. Test pins pass-through for inline, autolink, reference, HTML, JSX and fenced links.@11ty/gray-matter+js-yaml4 (both already installed transitively;npm ciadds no packages). YAML errors now fail with the file name.titleFromFilename()keeps acronyms (QDP, API, CUDA, ...); explicit titles and# Headingbodies are never overridden.docusaurus.config.ts: function-formeditUrlfor docs and blog..github/workflows/website-build.yml: runnpm test.website/README.md: sync rules; fix the Blog Posts location.dev/adr/002-docs-source-layout.md: proposed ADR, Decision left open.Testing
npm test: 24/24 pass; the 6 new parser tests fail against the old script.qumat/api.mdandqdp/api.md).npm run buildsucceeds on main and branch with the same warnings; all 250 HTML pages identical apart from edit links. 226/226 edit URLs resolve to tracked files.package-lock.json: 2 lines for the new deps plus 20"dev": truelines that npm also produces on untouched main.npm run version.🤖 Generated with Claude Code
https://claude.ai/code/session_01GuXmXRXdNF43h2FoQcNAZb