docs: split integration docs and restore docs mode#4
Conversation
baeaa68 to
6a97c0b
Compare
ethanj
left a comment
There was a problem hiding this comment.
Review of PR #4 (docs: split integration docs and restore docs mode)
The integration docs split (overview / local / cloud per integration) is a good restructure, the new DocsMode flag is clean, and the slug strategy (slug: /integrations/coding-agents/claude-code/local) keeps URLs stable. But the PR ships three regressions against AFDocs work merged in PR #5 that will break the live build and degrade the agent-facing markdown — these need to land before merge.
I checked out the branch (6a97c0b), ran npm install, then npm run build.
🔴 Blocker 1 — AFDocs runtime deps removed from package.json
6a97c0b deletes cheerio, turndown, turndown-plugin-gfm, js-yaml, @types/js-yaml, and @types/turndown from devDependencies. Those are required by scripts/mirror-markdown.mjs (added in PR #5 to render the .md mirror from built HTML for AFDocs markdown-content-parity).
$ npm run build
[ERROR] Error: Docusaurus could not load module at path
".../src/plugins/llms-and-mirror-plugin.mjs"
[cause]: Error: Cannot find module 'turndown'
Require stack:
- .../scripts/mirror-markdown.mjs
Fix: keep all six entries in devDependencies from origin/main's package.json. The package-lock.json should also retain them.
I suspect the deps got dropped during a package.json formatting/dedupe pass alongside the version bumps to ^3.10.1 — easy to drop accidentally.
🔴 Blocker 2 — prebuild / prestart auto-regen reintroduced
6a97c0b adds back:
"write:docs-mode": "node scripts/write-docs-mode.mjs",
"prestart": "npm run write:docs-mode && npm run regen:api",
"prebuild": "npm run write:docs-mode && npm run regen:api",PR #5 explicitly removed the prebuild/prestart auto-regen of regen:api (commit 08c5312) because it dirtied 29 committed .api.mdx files on every build (non-deterministic re-encoding of the compressed api: blob). I just reproduced that regression on this branch:
$ git status -sb
M docs/api-reference/http/consolidate-memories.api.mdx
M docs/api-reference/http/ingest-memory-quick.api.mdx
M docs/api-reference/http/ingest-memory.api.mdx
M docs/api-reference/http/search-memories-fast.api.mdx
M docs/api-reference/http/search-memories.api.mdx
M docs/api-reference/http/update-config.api.mdx
Fix: keep write:docs-mode as a script (it's needed) but stop calling regen:api from prebuild / prestart. Two clean options:
"prestart": "npm run write:docs-mode",
"prebuild": "npm run write:docs-mode",…or fold write:docs-mode directly into start / build if you want a single-call shape. Either way: don't run regen:api automatically. Spec refresh stays the explicit npm run vendor:spec && npm run regen:api && commit flow that scripts/vendor-core-spec.mjs:14 already documents.
🔴 Blocker 3 — src/theme/Heading swizzle breaks .md mirrors
The new <Heading> component wraps every heading's text in an anchor link with a # icon span. Rendered HTML:
<h1 class="anchor anchorTarget_QU3I" id=observability>
<a href=#observability class=headingLink_no4V title="Copy link to Observability">
<span aria-hidden=true class=headingIcon_Pk3T>#</span>
<span class=headingText_IKAy>Observability</span>
</a>
</h1>scripts/mirror-markdown.mjs reads each route's HTML, strips known chrome (breadcrumbs, TOC, copy buttons, a.hash-link) before turndown, then converts to markdown. scripts/mirror-markdown.mjs:147-159 strips a.hash-link, but the new component uses CSS-module class names (headingLink_no4V, headingIcon_Pk3T) — different class, same purpose. So turndown converts the whole anchor + icon as part of the heading text:
$ head -2 build/platform/observability.md
# [#Observability](#observability "Copy link to Observability")
Every heading on every hand-authored page now reads that way. AFDocs's markdown-content-parity check tolerates this (HTML and md both render the link), but agents reading the mirror see broken-looking headings with a stray # glued to the title text. It also pushes the H1 text past where the remark plugin's directive injection looks for it (the plugin matches node.type === 'heading' && depth === 1 on the AST, so injection still works at parse time — but anything that operates on the rendered .md will see noise).
Fix: in src/theme/Heading/index.tsx, either:
- Add
data-markdown-ignoreto the icon span (AFDocs strips it before parity comparison; my mirror could also use it as a strip selector), or - Add the
hash-linkclass to the wrapping<a>(compat with default Docusaurus selectors that other tooling assumes), or - Render the icon span via CSS pseudo-element (
::before) so it's never in the DOM text content.
Option 3 is cleanest — keeps the hover affordance, drops the # from the converted text, doesn't require mirror-markdown.mjs changes.
If you'd rather keep the JSX structure, add the icon class to the noise selector list:
// scripts/mirror-markdown.mjs:146
const ARTICLE_NOISE_SELECTORS = [
…,
"a.hash-link",
"[class*='headingIcon']", // ← add
"[class*='headingLink']", // ← also strip the wrapping link
…
];…and update the findInsertIndex / heading conversion in turndown to keep just the headingText span content. That works but couples the mirror to theme internals.
🟡 Concern 4 — collision with PR #6 (round 3)
PR #6 (open at #6) is the round-3 AFDocs follow-up:
- Adds the
Agent index: [llms.txt](/llms.txt)directive to OpenAPI mirrors - Drops
.well-known/mcp.jsonfrom llms.txt's "Optional" section
It touches scripts/build-llms-txt.mjs and scripts/mirror-markdown.mjs — both files PR #4 also modifies (for blocker 1's revert). Whichever PR merges second will need a small rebase. No deep conflict, just heads-up.
🟡 Concern 5 — <PublishedOnly> / <SourceOnly> interaction with markdown mirrors
The mirror reads built HTML, so it captures only the active ATOMICMEMORY_DOCS_MODE's branch. If you build with ATOMICMEMORY_DOCS_MODE=published, the .md mirror has only published content; if you build with source, it has only source content. That's probably fine — we want the mirror to match what users see — but worth being explicit:
- The published deploy at
docs.atomicmemory.aidecides which branch shows up in the mirror. - If you ever ship two deploys (one published, one source), only one will have correct mirrors.
- A note in
static/skill.mdexplaining the asymmetry would help agents that hit a "the docs say X but the .md says Y" mismatch.
✅ What I liked
- Slug strategy:
slug: /integrations/coding-agents/claude-code/localkeeps URL hierarchy clean while letting the source files stay flat. My llms.txt section map keys off URL prefixes, so this works without changes. DocsModevalue component: clean React shape (PublishedOnly/SourceOnly/DocsModeValuewithpublished/sourceslots). Easy for authors to use.onBrokenAnchors: 'throw': good catch — pairs well with the existingonBrokenLinks: 'throw'and would have caught some cross-link drift early.- Heading anchor UX: copy-link-on-click is a real improvement for humans; just needs the markdown-conversion concession above.
- Logo treatment:
srcDarkforlogo-full-white.svgis the right Docusaurus pattern. - OpenAPI integration page: the new
openai-agentspage is a meaningful addition.
Suggested order
- Restore the six removed deps in
package.json+package-lock.json(blocker 1). - Drop
regen:apifromprebuild/prestart; keepwrite:docs-mode(blocker 2). - Pick a heading-fix approach (blocker 3) — option 3 (CSS pseudo-element) is the lowest-coupling path.
- Land PR #6 first (it's small and orthogonal), then rebase this PR on the result.
- Re-run
npx afdocs check https://docs.atomicmemory.ai --format scorecardagainst the deploy preview to confirm the score didn't regress.
Happy to send these as a follow-up commit on the branch if useful — let me know.
…in mirror Three blockers caught in PR review against the AFDocs work merged in PR #5. Each lands with a focused fix. ## Blocker 1: AFDocs runtime deps removed from package.json `6a97c0b` dropped `cheerio`, `turndown`, `turndown-plugin-gfm`, `js-yaml`, `@types/js-yaml`, `@types/turndown` from devDependencies. Those are required by `scripts/mirror-markdown.mjs`, so `npm run build` fails with `Cannot find module 'turndown'` from the `llms-and-mirror-plugin.mjs` postBuild path. Restored all six in devDependencies (matching origin/main's pinned versions). ## Blocker 2: prebuild/prestart auto-regen reintroduced `6a97c0b` re-added `prestart`/`prebuild` hooks that run `write:docs-mode && regen:api`. PR #5's commit `08c5312` explicitly removed `regen:api` from those hooks because it dirtied the 29 committed `.api.mdx` files on every build (non-deterministic re-encoding of the compressed `api:` blob). Verified the regression by running `npm run build` on this branch and seeing 6 .api.mdx files modified in the worktree afterward. Kept `write:docs-mode` (it's needed for the new docs-mode flag) but dropped the `regen:api` chain. Spec refresh stays the explicit `vendor:spec → regen:api → commit` flow that `scripts/vendor-core-spec.mjs:14` already documents. ## Blocker 3: custom Heading swizzle leaked into the markdown mirror The new `<Heading>` component (`src/theme/Heading/index.tsx`) wraps each heading's text in an anchor link with a `#` icon span — a copy- to-clipboard affordance for humans. The wrapping anchor uses CSS-module class names (`headingLink_no4V`, `headingIcon_Pk3T`) which the mirror's noise selector strips list (which targets the default `a.hash-link`) doesn't match. Result: every heading in every mirror rendered as # [#Observability](#observability "Copy link to Observability") instead of # Observability Two-part fix: 1. `src/theme/Heading/index.tsx` — add `data-markdown-ignore` to the `#` icon span. AFDocs treats this attribute as the spec-compliant marker for HTML-only content that should not appear in markdown parity comparison; tooling that converts the page to markdown should also strip it. 2. `scripts/mirror-markdown.mjs` — - Add `[data-markdown-ignore]` to `ARTICLE_NOISE_SELECTORS` so the icon span is removed before turndown runs (defense for other tooling that might emit the same attribute). - Add a `clean-heading-text` turndown rule that intercepts `<h1>`–`<h6>` and emits clean `# Title` from `node.textContent` (with leading `#` chars stripped). This works for both default Docusaurus headings and the swizzled component, so the mirror no longer carries the wrapping anchor link as part of the heading text. Verified: `npm run build` produces clean `# Observability` / `## The summary shapes` in `build/platform/observability.md`. ## Verification - `npm run typecheck`: pass - `npm run build`: pass; worktree has 0 .api.mdx files dirtied (down from 6 before this commit) - 7/7 platform pages + 31/31 API reference pages have the llms.txt directive in HTML - All four AFDocs artifacts present: `llms.txt`, `llms-full.txt`, `skill.md`, `.well-known/mcp.json` - Sample mirror (`build/platform/observability.md`) shows clean ATX headings with no anchor-link leak ## Note for the round-3 PR PR #6 also touches `scripts/mirror-markdown.mjs` and `scripts/build-llms-txt.mjs`. The two PRs don't overlap on the same lines, but whichever lands second will need a small rebase.
6089df8 to
c7ed03e
Compare
Summary
Validation