Skip to content

fix(docs): flatten tab JSX before the LLM plugin reads it, not after - #451

Closed
allxsmith wants to merge 2 commits into
mainfrom
fix/llms-flatten-before-plugin
Closed

allxsmith wants to merge 2 commits into
mainfrom
fix/llms-flatten-before-plugin

Conversation

@allxsmith

@allxsmith allxsmith commented Aug 1, 2026 •

Copy link
Copy Markdown
Owner

Pull Request

Description

Fixes a live content-loss bug on main: tab labels are missing from the published LLM
artifacts right now. Found while rebasing #433, but it stands alone and should land first.

docusaurus-plugin-llms 0.5.0 added an unconditional strip of PascalCase JSX tags
(src/content.ts):

// Remove MDX/JSX component tags (PascalCase element names such as <Tabs>,
// <TabItem>, <Admonition>), keeping their inner text content.
cleaned = cleaned.replace(/<\/?[A-Z][A-Za-z0-9.]*\b[^>]*\/?>/g, '');

Keeping inner text is not keeping props. Content carried in an attribute goes with the tag:

  • <TabItem label="…"> keeps its body but loses its label, so the #### headings the
    flattener emits never existed.
  • <PackageManagerTabs command="…" /> is self-closing — its entire payload is the prop, so
    the command vanishes completely.

Both are silent. flatten-llms-tabs.mjs ran after the build, and verifyArtifact looks for
JSX that survived, never for content that vanished — so the build stayed green while
the machine-read copy of the docs quietly lost information. #447 bumped the plugin 0.4.0 →
0.5.1 and was green because no page used PackageManagerTabs yet; the TabItem half has been
broken since it merged.

Live evidence on main — App root (global) is a TabItem label in skills/theming.mdx:

source llms-full.txt / per-page twin
before 1 0
after 1 1

The fix is ordering, not content

scripts/prepare-llms-source.mjs (chained ahead of docusaurus build) mirrors docs/ into a
gitignored .llms-src/ with the tabs already flattened. The plugin reads that mirror via

docsDir: [{ path: '.llms-src', routeBasePath: 'docs' }]

a supported shape — path is the directory it reads, routeBasePath is what URLs are built
from — so published links are unchanged. By the time the plugin runs there is no PascalCase JSX
left, and its strip becomes a no-op instead of a lossy step. transform() itself is unchanged;
only when it runs moved.

Verification also moves earlier, so a failure now names the source file an author can fix
rather than a generated artifact path.

  • bulma-ui (@allxsmith/bestax-bulma)
  • create-bestax (create-bestax)
  • docs (@allxsmith/bestax-docs)

Related Issue(s)

Refs #447
Refs #433
Refs #418

Type of Change

  • Bug fix
  • Build tooling

Alternatives considered

Pinning the plugin back to 0.4.0 would also work — 0.4.0 has no such strip, and its caret
ranges ("minimatch": "^9.0.3" vs 0.5.1's exact "9.0.3") would let the minimatch@9/yaml@2
overrides from #447 be pruned. Rejected: staying on current dependencies is the house rule, and
upstream's direction — plain-text artifacts shouldn't contain JSX — is defensible. This change
works with that direction rather than against it.

Checklist

  • My code follows the project style guidelines
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective
  • I have added/updated documentation as needed
  • All new and existing tests passed

Verification

pnpm --filter @allxsmith/bestax-docs test — 83 pass (77 existing + 6 new).
prepare-llms-source.test.mjs targets the failure class that let this ship green:

  • a self-closing PackageManagerTabs becomes a real pnpm fence
  • a TabItem label survives as a heading, not just its body
  • the mirror is rebuilt from scratch, so a deleted page stops appearing in llms.txt
  • prepare refuses a source file the flattener cannot handle, naming it
  • the real 144-page corpus leaves the plugin nothing to strip
  • the plugin's own regex is applied to the mirror and proven to delete nothing

Also checked by hand:

  • pnpm run check:urls passes; llms.txt links still resolve to https://bestax.io/docs/…
  • 144 documents processed, unchanged
  • Control build: reverting docsDir to its old string form makes the labels disappear
    again, confirming the mechanism.
  • The one .md twin that lands outside /docs/ (the migration index page, which sets an
    explicit slug:) does so identically before and after — pre-existing plugin behaviour,
    not caused by this change. Worth its own look sometime.

Additional Context

I have not filed this upstream yet. It is arguably a real bug — "keeping their inner text
content" is the stated rationale, and prop-borne content isn't covered by it — and there are
currently zero open issues on that repo with a responsive maintainer. Happy to file a minimal
repro if you want; this PR is independent of the outcome either way.

Summary by CodeRabbit

  • Documentation

    • Improved documentation build processing for tabbed content while preserving commands, labels, code samples, and tab bodies.
    • Published documentation routes remain unchanged.
    • Added validation to catch unsupported inline formatting and content-processing issues before publication.
    • Generated documentation artifacts are automatically refreshed and excluded from formatting checks.
  • Tests

    • Expanded coverage for tab rendering, content preservation, stale generated content cleanup, and full documentation validation.

docusaurus-plugin-llms 0.5.0 added an unconditional strip of PascalCase JSX
tags:

    // Remove MDX/JSX component tags (PascalCase element names such as <Tabs>,
    // <TabItem>, <Admonition>), keeping their inner text content.
    cleaned = cleaned.replace(/<\/?[A-Z][A-Za-z0-9.]*\b[^>]*\/?>/g, '');

Keeping *inner text* is not keeping *props*. Content carried in an attribute
goes with the tag, which broke two things the moment the plugin was bumped:
`<TabItem label="…">` kept its body but lost its label, so the #### headings
the flattener emits never existed; and a self-closing
`<PackageManagerTabs command="…" />` would lose its command entirely.

Both are silent. flatten-llms-tabs.mjs ran *after* the build and verifyArtifact
looks for JSX that survived, never for content that vanished — so the build
stayed green while the machine-read copy of the docs lost information. The
TabItem half is live on main today: "App root (global)" appears in
skills/theming.mdx and in zero build artifacts.

The fix is ordering, not content. prepare-llms-source.mjs mirrors docs/ into a
gitignored .llms-src/ with the tabs already flattened, and the plugin reads
that via docsDir: [{ path: '.llms-src', routeBasePath: 'docs' }] — a supported
shape where path is the directory read and routeBasePath is what URLs are built
from, so published links are byte-identical. By the time the plugin runs there
is no PascalCase JSX left, and its strip becomes a no-op instead of a lossy
step. Verification also moves earlier, so a failure names the source file an
author can fix rather than a generated artifact.

Verified: with docsDir reverted to its old string form the artifacts lose the
labels again, and the one .md twin that lands outside /docs/ (the migration
index page, which sets an explicit slug) does so identically before and after —
pre-existing, not caused by this change.

prepare-llms-source.test.mjs covers the failure class that let this ship: it
asserts prop-borne content is materialized, runs the real corpus through the
mirror, and applies the plugin's own regex to prove it can no longer delete
anything.

Claude-Session: https://claude.ai/code/session_01TGA6sFTUGsJ6oXhfpjKEnh
@coderabbitai

coderabbitai Bot commented Aug 1, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9f0e2646-4339-4c88-b11f-13a50f3f2470

📥 Commits

Reviewing files that changed from the base of the PR and between 32ac149 and 3281a25.

📒 Files selected for processing (1)
  • .prettierignore

Walkthrough

The documentation build now prepares a flattened .llms-src mirror before Docusaurus runs. The LLM plugin reads this mirror while published routes remain under docs. Tests validate transformation, cleanup, corpus coverage, and plugin compatibility.

Changes

LLM documentation build

Layer / File(s) Summary
Prepare and validate the documentation mirror
docs/scripts/prepare-llms-source.mjs, docs/scripts/prepare-llms-source.test.mjs
The preparation script rebuilds .llms-src, transforms Markdown and MDX files, verifies output, and reports failures. Tests cover tab flattening, cleanup, and unsupported JSX.
Wire preparation into the Docusaurus build
docs/package.json, docs/docusaurus.config.js, docs/.gitignore, .prettierignore, docs/CLAUDE.md
The build prepares the mirror before Docusaurus. The plugin reads .llms-src and publishes routes under docs. Ignore rules exclude the generated mirror.
Validate corpus and plugin compatibility
docs/scripts/prepare-llms-source.test.mjs
Tests validate the full documentation corpus and confirm that the plugin JSX-strip regex is a no-op on prepared files.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Build as Documentation build
  participant Prepare as prepare-llms-source.mjs
  participant Plugin as LLM documentation plugin
  participant Routes as docs routes
  Build->>Prepare: Create .llms-src
  Prepare->>Plugin: Provide flattened documentation
  Plugin->>Routes: Publish under docs
Loading

Possibly related PRs

Suggested labels: deep-review

Suggested reviewers: bestaxbot

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary fix: flattening tab JSX before the LLM plugin processes documentation.
Description check ✅ Passed The description explains the bug, affected docs package, implementation, tests, verification results, and related issues.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/llms-flatten-before-plugin

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Preview Deployment

Preview URL: https://62ebb98c.bestax.pages.dev

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@docs/scripts/prepare-llms-source.test.mjs`:
- Around line 147-156: Update the corpus test around transform and offenders to
also apply the real PLUGIN_JSX_STRIP regex to each mirrored file and assert that
it produces no changes. Keep the existing transform check, and ensure the
assertion reports the relative file path for any page containing strip-eligible
PascalCase JSX.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 56d84fbd-82eb-4d12-b6df-c6f96b91f469

📥 Commits

Reviewing files that changed from the base of the PR and between e0af2d0 and 32ac149.

📒 Files selected for processing (6)
  • docs/.gitignore
  • docs/CLAUDE.md
  • docs/docusaurus.config.js
  • docs/package.json
  • docs/scripts/prepare-llms-source.mjs
  • docs/scripts/prepare-llms-source.test.mjs

Comment on lines +147 to +156
for await (const file of walk(target)) {
const body = await readFile(file, 'utf8');
// `transform` is the authority on what counts as code; re-running it is a
// no-op on an already-flattened file, so any tab tag it would still act on
// is one that reached the mirror outside code.
if (transform(body) !== body) {
offenders.push(path.relative(target, file));
}
}
assert.deepEqual(offenders, [], 'tab JSX reached the mirror unflattened');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Apply PLUGIN_JSX_STRIP to the real corpus mirror.

The corpus test only checks whether transform(body) is a no-op. It does not verify that the plugin regex is a no-op for every mirrored page. A remaining PascalCase JSX tag outside the tab transform scope can still be stripped during the production build.

Proposed test update
-      if (transform(body) !== body) {
+      if (
+        transform(body) !== body ||
+        body.replace(PLUGIN_JSX_STRIP, '') !== body
+      ) {
         offenders.push(path.relative(target, file));
       }
     }
-    assert.deepEqual(offenders, [], 'tab JSX reached the mirror unflattened');
+    assert.deepEqual(
+      offenders,
+      [],
+      'plugin-stripable JSX reached the mirror'
+    );

As per coding guidelines, maintain tests that guarantee the plugin’s JSX-stripping regex is a no-op against the mirror.

📝 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
for await (const file of walk(target)) {
const body = await readFile(file, 'utf8');
// `transform` is the authority on what counts as code; re-running it is a
// no-op on an already-flattened file, so any tab tag it would still act on
// is one that reached the mirror outside code.
if (transform(body) !== body) {
offenders.push(path.relative(target, file));
}
}
assert.deepEqual(offenders, [], 'tab JSX reached the mirror unflattened');
for await (const file of walk(target)) {
const body = await readFile(file, 'utf8');
// `transform` is the authority on what counts as code; re-running it is a
// no-op on an already-flattened file, so any tab tag it would still act on
// is one that reached the mirror outside code.
if (
transform(body) !== body ||
body.replace(PLUGIN_JSX_STRIP, '') !== body
) {
offenders.push(path.relative(target, file));
}
}
assert.deepEqual(
offenders,
[],
'plugin-stripable JSX reached the mirror'
);
🤖 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 `@docs/scripts/prepare-llms-source.test.mjs` around lines 147 - 156, Update the
corpus test around transform and offenders to also apply the real
PLUGIN_JSX_STRIP regex to each mirrored file and assert that it produces no
changes. Keep the existing transform check, and ensure the assertion reports the
relative file path for any page containing strip-eligible PascalCase JSX.

Source: Coding guidelines

@allxsmith
allxsmith requested review from bestaxbot and Copilot August 1, 2026 18:07
@allxsmith

Copy link
Copy Markdown
Owner Author

Filed upstream

rachfop/docusaurus-plugin-llms#64 — "JSX tag stripping silently drops content carried in props (0.5.0+)".

It reports both cases with minimal repros: the <TabItem label> one (which is what is live on
main today) and the self-closing prop-carrying one. It offers three directions rather than a
demand — a preserveComponents option, a content hook that runs before cleaning, or emitting
label/value as a heading for the <Tabs> case the code comment already names — and offers
a PR for whichever they prefer. The workaround this PR implements is written up at the end, so
anyone else hitting it has a way out.

This PR does not depend on the outcome. Flattening before the plugin is correct regardless:
it works with upstream's stated direction (plain-text artifacts should not contain JSX) rather
than against it. If they add a hook, the mirror can be swapped for it later without changing
transform() or any authoring convention — only where it runs would move again.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a production docs ingestion regression where docusaurus-plugin-llms started stripping PascalCase JSX tags (v0.5.0+), causing tab label / prop-borne content to silently disappear from published LLM artifacts. The change moves tab-flattening to a pre-build “mirror” directory (.llms-src) that the plugin reads, making the plugin’s strip step non-lossy for tabs.

Changes:

  • Add scripts/prepare-llms-source.mjs to build a flattened .llms-src/ mirror and run source-level verification before docusaurus build.
  • Point docusaurus-plugin-llms at the mirror via docsDir: [{ path: '.llms-src', routeBasePath: 'docs' }] to preserve published URLs.
  • Add targeted tests to prevent regressions (self-closing PackageManagerTabs, TabItem label preservation, stale mirror cleanup, and early failure on unsupported inline shapes).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
docs/scripts/prepare-llms-source.mjs New pre-build mirroring step that flattens tabs and verifies output before the plugin reads it.
docs/scripts/prepare-llms-source.test.mjs New tests guarding against the specific “props lost by JSX stripping” failure mode.
docs/docusaurus.config.js Repoints the LLM plugin to read the flattened mirror directory while keeping route base paths stable.
docs/package.json Chains the new prepare step ahead of docusaurus build.
docs/CLAUDE.md Updates docs pipeline documentation to reflect the new ordering-based fix.
docs/.gitignore Ignores the generated .llms-src mirror directory.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

);
});

test('the real corpus leaves the plugin nothing to strip', async () => {
Comment on lines +53 to +56
export async function prepare(sourceDir, targetDir) {
// Rebuild from scratch so a renamed or deleted page can't linger in the mirror
// and keep appearing in llms.txt after it is gone from the site.
await rm(targetDir, { recursive: true, force: true });
Comment thread docs/CLAUDE.md
Comment on lines +43 to +44
Covered by `scripts/flatten-llms-tabs.test.mjs` and `scripts/prepare-llms-source.test.mjs`, the
latter asserting the plugin's own regex is a no-op against the mirror (`pnpm test`).
CI runs `format:check` after `build`, so the mirror written by
prepare-llms-source.mjs exists by then and prettier lints it as if it were
source. It is generated from already-formatted pages but the flattening
reflows them, so one file came back unformatted and reddened Build and Test.

.gitignore does not help — prettier reads .prettierignore only.

Claude-Session: https://claude.ai/code/session_01TGA6sFTUGsJ6oXhfpjKEnh
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Preview Deployment

Preview URL: https://e1d9c33c.bestax.pages.dev

@allxsmith

Copy link
Copy Markdown
Owner Author

Superseded by a simpler approach — closing in favour of authoring the pnpm block as the component's children rather than a prop.

The plugin strips tags but keeps inner text, so a fenced block inside the component survives untouched. Verified by running the real cleanMarkdownContent from 0.5.1 over both shapes:

authoring plugin output
<PackageManagerTabs command="add foo" /> ### Install → ### Next — content gone
fenced block as children ### Install → bash pnpm add foo → ### Next — exactly right

That makes the artifact correct by construction rather than correct by pipeline, and it removes everything this PR added: the mirror, the docsDir indirection, the extra build step, and the two ignore entries. Strictly less machinery for the same result on the component we own.

Known trade-off, accepted: this does not help <TabItem label="…">, because label is Docusaurus's own component API and can't move to children. Those labels stay missing from the artifacts on skills/theming.mdx and skills/custom-component.mdx — <Tabs> collapses to "body one\n\nbody two" with nothing distinguishing the options. Tracked by the upstream issue and worth fixing separately.

Upstream rachfop/docusaurus-plugin-llms#64 stays open and still describes both cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants