Skip to content

chore(docs): make the LLM flattener indentation-aware before the #402 rollout - #418

Merged
allxsmith merged 3 commits into
mainfrom
docs/pm-tabs-flattener-indent
Jul 30, 2026
Merged

allxsmith merged 3 commits into
mainfrom
docs/pm-tabs-flattener-indent

Conversation

@allxsmith

@allxsmith allxsmith commented Jul 30, 2026 •

Copy link
Copy Markdown
Owner

First PR of #402. No docs pages change here — this fixes a latent defect in the flattener that #408 shipped, which would have corrupted the LLM artifacts the moment a <PackageManagerTabs> landed inside a numbered step.

The bug

The flattener replaces JSX in place, but only the opening fence inherited the tag's indentation. Running the merged transform() on a nested component:

1. **Create it:**

   <PackageManagerTabs command="add foo" />

produced:

1. **Create it:**

   ```bash
pnpm add foo

Opening fence at 3 spaces, body and closer at column 0. Per CommonMark the under-indented lines terminate the list item: the command becomes prose, and the following step gets swallowed into the block that the stray ``` opens. `transform()` reports the corrupted output as idempotent, so nothing self-heals, and re-running masks the damage as code.

**20 of #402's target sites are nested** — 18 in `react-setups.md` at 3 spaces, `migration/bulma-ui-3-to-4.md:40` at 2 — and because `llms-full.txt` is one concatenated document, `bulma-ui-3-to-4.md` (the only fence on its page) would have bled into the next page. Nothing in CI would have caught it: the HTML site builds perfectly and only the machine-readable artifacts are wrong.

## The fix

Both `flattenPackageManagerTabs` and `flattenGenericTabs` now capture the tag's indentation and carry it onto every emitted line. Two consequences worth reviewing:

- The pattern is **anchored to the start of a line**, since only a tag that starts its own line has an indentation to inherit. A component mid-sentence therefore no longer matches and survives as visible JSX — a loud failure instead of an inline fence that corrupts the document. The build gate below catches it.
- `flattenGenericTabs` **dedents then re-indents** the tab body rather than prefixing it, so a body that already carries the source indentation doesn't end up doubly indented — at 4+ spaces that would silently become a markdown code block.

## Shared translation module

Adds `docs/src/components/PackageManagerTabs/translate.mjs` — the pnpm → npm/yarn/bun vocabulary, which the component in the next PR will consume. The flattener now imports `renderCommand` from it instead of its own `toPnpm`, so the "duplicating them here would be a second source of truth" claim in its header comment is mechanically true rather than aspirational. A test asserts the flattened body equals `renderCommand(cmd, 'pnpm')` for every row of the table — the single assertion keeping llms.txt and the default tab from drifting.

`.mjs` because `docs/package.json` has no `"type": "module"`, so node would load a `.js` as CommonJS and `node --test` couldn't import it.

A `command` may now hold several `;`-separated lines. Segments whose first token is a known verb get translated; anything else (`cd my-app`, a `#` comment) passes through byte-identically. That's what the Quick Start scaffold sequences need.

## Build gate

`main()` now throws if a rewritten artifact still contains tab JSX outside code, has an unterminated fence, or has **a fenced block dedented below its opening fence**.

That third check is the one that catches the bug above, and it's worth explaining why the obvious check doesn't: I first wrote this as a fence-parity assertion and the test failed. CommonMark lets a closing fence sit at 0–3 spaces *regardless* of the opening indent, so the corrupted output pairs its fences happily and parity sees nothing wrong. The damage is structural, not lexical. Comparing body indentation against the opening fence is what actually detects it.

## Verification

- **The LLM artifacts are byte-identical to a pre-change baseline build.** `diff -r` over `*.md` twins and `llms*.txt` is empty, so this closes the latent defect without moving any published output.
- Coverage 21 → **64 tests**. The new ones were written failing first against the merged code: 11 failed before the fix.
- Includes a **corpus test** that runs every shipped page through `transform()` and asserts no surviving tab JSX, no dedented or unterminated fence, no empty emitted block, and idempotency — so a future nested usage fails in `pnpm test` rather than silently in `llms-full.txt`.
- Gates: `check:conformance` 7/7, `gen:catalog:check` clean, prettier clean, full docs build green with the new verifier active.

One incidental cleanup: removed a duplicated `outsideCode` doc comment that #408 left sitting above `maskFences`.

## Not in scope

The component, the global `MDXComponents` registration, the homepage, and any docs page conversion — those follow in the next PRs of #402.


<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

* **New Features**
  * Documentation now renders package-manager commands consistently for pnpm, npm, Yarn, and Bun.
  * Multi-step commands are displayed clearly across separate lines.
  * Flattened documentation preserves indentation, list formatting, headings, and code blocks.
* **Bug Fixes**
  * Improved handling of tabbed documentation and nested code examples.
  * Build validation now detects unflattened tabs, malformed fences, and swallowed commands before release.
* **Tests**
  * Added comprehensive coverage for documentation transformations and command rendering.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

…lout

The flattener replaces JSX in place but emitted its fence body and closing
fence at column 0, keeping only the opening fence's indentation. For a
component nested in a list item that produces:

       ```bash        <- 3 spaces, opens inside the <li>
    pnpm add foo       <- column 0, terminates the list item
    ```                <- column 0

The command becomes prose and the following step gets swallowed. 20 of the
#402 target sites are nested (18 in react-setups.md at 3 spaces,
bulma-ui-3-to-4.md at 2), so this had to land before any of them.

Both flattenPackageManagerTabs and flattenGenericTabs now capture the tag's
indentation and carry it onto every emitted line. The pattern is anchored to
the start of a line, so a component sitting mid-sentence no longer matches and
survives as visible JSX rather than an inline fence that corrupts the document.

Adds the shared translation module the component will use. The flattener now
imports renderCommand from it instead of its own toPnpm, so the "no second
source of truth" property in its header comment is mechanically true — and a
test asserts the flattened body equals the pnpm tab for every table row. A
`command` may now hold several `;`-separated lines, which is what the Quick
Start scaffold sequences need.

Adds a build gate: main() throws if a rewritten artifact still contains tab JSX
outside code, has an unterminated fence, or has a fenced block dedented below
its opening fence. That last check is the one that catches the bug above —
CommonMark pairs those fences happily, so parity alone sees nothing wrong.

Coverage goes 21 -> 64, including a corpus test that runs every shipped page
through transform() and asserts the invariants, so a future nested usage fails
here instead of silently in llms-full.txt.

Verified: the LLM artifacts are byte-identical to a pre-change baseline build,
so this closes the latent defect without moving any published output.
@coderabbitai

coderabbitai Bot commented Jul 30, 2026 •

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The change adds shared package-manager command translation, updates tab flattening to preserve indentation and Markdown structure, strengthens fenced-code and residual-JSX verification, and adds unit, gate, and full-documentation-corpus tests.

Changes

Documentation flattening and verification

Layer / File(s) Summary
Package-manager command translation
docs/src/components/PackageManagerTabs/translate.mjs, docs/scripts/package-manager-translate.test.mjs
Adds command splitting, pnpm/npm/yarn/bun translation, rendering, linting, and coverage for special cases and passthrough behavior.
Indent-aware tab flattening
docs/scripts/flatten-llms-tabs.mjs, docs/scripts/flatten-llms-tabs.indent.test.mjs
Preserves JSX indentation when flattening package-manager and generic tabs, renders multi-segment commands, promotes tab labels to headings, and validates idempotent output.
Artifact verification and build gate
docs/scripts/flatten-llms-tabs.mjs, docs/scripts/flatten-llms-gate.test.mjs, docs/scripts/flatten-llms-tabs.indent.test.mjs
Adds list-aware fence parsing, reports residual JSX and malformed fences, and aggregates verification failures across qualifying build artifacts.
Documentation corpus integration
docs/scripts/flatten-llms-corpus.test.mjs
Applies flattening and verification across Markdown and MDX documentation pages, checking pass-through behavior, empty bash blocks, and idempotency.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

  • allxsmith/bestax#408: Extends and tests the same flatten-llms-tabs.mjs flattener and its documentation build integration.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: making the docs LLM flattener indentation-aware for the #402 rollout.
Description check ✅ Passed The description covers the change summary, affected docs area, related issue context, type, tests, and remaining scope.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/pm-tabs-flattener-indent

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.


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

Copy link
Copy Markdown
Contributor

Preview Deployment

Preview URL: https://2ba556ab.bestax.pages.dev

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

This PR hardens the docs LLM-artifact flattener ahead of the #402 tabs rollout by making the JSX-to-Markdown rewrite indentation-aware (so list nesting isn’t corrupted), centralizing package-manager command translation in a shared module, and adding a verification gate + broader test coverage to catch silent artifact corruption.

Changes:

  • Update flatten-llms-tabs.mjs to preserve indentation on all emitted lines and add verifyArtifact() checks for residual tab JSX and fence-structure corruption.
  • Add shared translate.mjs for pnpm→npm/yarn/bun command rendering, and tests to keep the flattener’s emitted pnpm output in sync with the shared translator.
  • Add targeted indentation tests plus a corpus-wide test to prevent future docs shapes from silently breaking llms*.txt / per-page .md twins.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
docs/src/components/PackageManagerTabs/translate.mjs Introduces a shared translation/rendering module for package-manager commands.
docs/scripts/package-manager-translate.test.mjs Unit tests for translation + assertion that the flattener emits the exact pnpm rendering from the shared module.
docs/scripts/flatten-llms-tabs.mjs Makes flattener indentation-aware, imports renderCommand, and adds artifact verification + build-fail gate.
docs/scripts/flatten-llms-tabs.indent.test.mjs Tests indentation propagation, multi-segment commands, idempotency, and verifier failure modes.
docs/scripts/flatten-llms-corpus.test.mjs Runs the flattener across the docs corpus and asserts key invariants (no residual JSX, no empty fences, idempotency).

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

Comment thread docs/scripts/flatten-llms-tabs.mjs
Comment thread docs/src/components/PackageManagerTabs/translate.mjs Outdated

@claude claude 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.

Deep review — 0 blocking · 2 advisory

# Severity Area Finding Location
1 🔵 Advisory Robustness Dedent gate is blind to fences opened at ≥4 spaces (two-digit ordered lists, nested lists) — maskFences only recognizes opens at 0–3 leading spaces, so open.indent is always ≤3 and the dedent check can never fire for deeper nesting. The flattener itself is correct at those depths; this is a hole in the safety net, not the fix. docs/scripts/flatten-llms-tabs.mjs:150,178
2 🔵 Advisory Robustness Dedent predicate flags any non-blank body line less-indented than the opening fence, so legitimately flush-left content inside an indented fenced block (a heredoc, ASCII art) would false-positive and fail the build/corpus test. Corpus is clean today, so no current impact. docs/scripts/flatten-llms-tabs.mjs:179

Overall: The change is sound and does exactly what it claims. I ran pnpm test (64/64 green), confirmed there are currently zero <PackageManagerTabs> usages in the corpus so the toPnpm -> renderCommand(cmd,'pnpm') swap is a genuine no-op for published output, and verified the generic-<Tabs> reindentBlock change is identity for every existing skills/API page (their TabItem bodies share a 0-space margin, so shared===0 && indent==='' short-circuits). The riskiest part is the new verifyArtifact build gate — not because it's wrong, but because it's a heuristic whose coverage is narrower than the flattener it guards; the human should focus there and decide whether the two gaps below are worth closing before the #402 rollout leans on it.

Residual risk: the addressed failure class is "flattener drops a tag's indentation -> silent list-item corruption in the concatenated artifacts." Ways it could still occur:

  • Nesting deeper than 3 spaces — refuted for correctness (the flattener carries the captured indent at any depth; the 6-space unit test passes), but the gate would miss a future regression there (finding #1). A downstream stray fence sometimes trips the unterminated-fence check instead, but that's incidental, not guaranteed in a concatenated doc.
  • Generic <Tabs> #### breaking out of a list — the fix carries indent onto the heading, but verifyArtifact has no heading-level dedent check, so only a fence inside the TabItem body would be caught, not the heading itself. Covered by unit + corpus tests, not by the gate.
  • Mid-line / unquoted / braced-template tags — refuted: the line-anchored regex leaves them as raw JSX and verifyArtifact's residual-JSX scan turns that into a loud build failure (tested).

🏄 Clean set, brah — the flattener now hugs its indentation all the way down the line instead of dumping the body out at column zero and wiping the next page. Artifacts stay byte-for-byte with the baseline, tests doubled, and the new build gate is a solid paddle-out even if it can't see past the 3-space break. No blockers in the water — ship it and catch the #402 rollout on the next set. 🤙

Two review findings, both valid.

main() verified only when transform() had changed the file. That exempted
exactly the shape the gate exists to catch: a mid-line tag, which the
line-anchored pattern deliberately skips, leaves the artifact untouched and so
never got verified. Confirmed against the previous commit — a mid-line
<PackageManagerTabs> exits 0 there and exits 1 now. Verification now runs for
every artifact matching NEEDS_FLATTENING; only the write is conditional.

The unit tests could not have caught this: verifyArtifact was already covered
and correct, the wiring around it was wrong. Added flatten-llms-gate.test.mjs,
which runs the real script over a temp build dir and asserts the process exit
code, including that the prefilter still spares unrelated malformed pages.

translate.mjs claimed passthrough segments were emitted "byte-identically",
but splitSegments trims and collapses whitespace. The guarantee is that a
passthrough line is the same on every tab, not that it matches the source byte
for byte; reworded, and pinned with a test.

70 tests, up from 64. LLM artifacts still byte-identical to the baseline build.
@allxsmith

Copy link
Copy Markdown
Owner Author

Both addressed in 84dcbea.

main() skipped verification on a no-op transform — fixed

Correct, and worse than it reads: this exempted precisely the shape the gate was built for. The line-anchored pattern deliberately skips a mid-line tag, which means transform() returns the input unchanged, which meant after === before short-circuited before verifyArtifact ever ran. The "loud and visible" failure I claimed in the PR description was silent.

Confirmed against the previous commit, same fixture both times:

OLD (a6aa4bf)  flatten-llms-tabs: rewrote 0 file(s)          exit 0
NEW (84dcbea)  llms-full.txt: unflattened tab JSX outside code: <PackageManagerTabs
               1 artifact(s) failed verification              exit 1

Verification now runs for every artifact matching NEEDS_FLATTENING; only the write stays conditional.

Worth noting why the existing tests missed it: verifyArtifact had unit coverage and was correct. The bug was in the wiring around it, which a pure-function test cannot see. So the new coverage is flatten-llms-gate.test.mjs, which runs the real script against a temp build dir and asserts the process exit code — plus a case pinning that the prefilter still spares unrelated malformed pages, so this doesn't turn every pre-existing unterminated fence into a build failure.

"byte-identically" was inaccurate — fixed

Also correct. splitSegments trims and collapses whitespace, so # Remove the old dep renders as # Remove the old dep. I took the comment fix rather than the behaviour fix: the normalization is deliberate (it's what lets authors pad around the ; separators), and the guarantee that actually matters is that a passthrough line is identical across the four tabs, not identical to the source. Reworded to say that explicitly, and pinned with a test asserting all four managers emit the same passthrough line.

Verification

70 tests, up from 64. LLM artifacts still byte-identical to the pre-change baseline build, so neither fix moved published output.

@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment

Preview URL: https://2d6112a0.bestax.pages.dev

@allxsmith

Copy link
Copy Markdown
Owner Author

deep-review: The prior review predates 84dcbea and never saw the gate fix. Be adversarial about the gate itself, not the flattener.

  1. verifyArtifact's wiring. 84dcbea made verification unconditional and added flatten-llms-gate.test.mjs, which shells out to the real script over a temp build dir and asserts exit codes. Does that test actually fail if the wiring regresses, or can it pass vacuously (wrong cwd, swallowed stderr, temp dir not seeded, exit code masked by the harness)? The original bug was invisible to pure-function tests — confirm the new test is not invisible in the same way.

  2. The two advisories you raised. Both survive at head. maskFences only matches opens at 0–3 spaces so open.indent is never >= 4; state plainly whether [Docs] Offer npm/pnpm/yarn/bun for every command — Tabs in the docs, small PM links on the homepage #402's 20 nested sites can ever produce a fence the gate structurally cannot see, and whether the corpus test compensates or shares the blind spot.

  3. The false-positive direction. The dedent predicate flags any less-indented non-blank body line. What shipped page shape would trip it and turn a valid doc into a red build?

  4. "Byte-identical to a pre-change baseline." Try to refute it — is the diff -r scope actually complete over llms*.txt and the .md twins?

  5. translate.mjs semantics. Multi-segment ; commands plus whitespace normalization: find an input where a passthrough segment differs across tabs, or where a verb-leading segment is translated when it should not be.

@claude claude 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.

Deep review — 0 blocking · 2 advisory

# Severity Area Finding Location
1 🔵 Advisory Robustness Gate is structurally blind to fences indented ≥4 spaces (open.indent can only ever be 1–3), so a deep-nested emission that regressed to a column-0 body can evade the dedent signal; scoped away from #402's sites (all ≤3 sp) docs/scripts/flatten-llms-tabs.mjs:150
2 🔵 Advisory Robustness Dedent predicate false-positives on valid CommonMark — a fence opened at 1–3 spaces with column-0 content (a legal code block) is flagged, which could redden a future build docs/scripts/flatten-llms-tabs.mjs:178

Overall: The change is sound and does what it claims. I chased the wiring bug the gate-fix commit (84dcbea) targets and confirmed it is genuinely closed: main() now runs verifyArtifact before the after === before short-circuit, and flatten-llms-gate.test.mjs is not vacuous — it shells out to the real script over a seeded temp dir, asserts result.ok === false on the no-op mid-line-tag shape, and matches the specific unflattened tab JSX / failed verification stderr strings, so a wiring regression (or a crash for any other reason) fails the assertion rather than passing silently. The byte-identical-to-baseline claim also holds for current content: there is no <PackageManagerTabs> in the tree yet, every real theme <Tabs>/<TabItem> is at column 0 (so reindentBlock is the identity), and the indented <Tabs> are bestax's own component inside tsx fences (masked). The riskiest part is the gate's own heuristics, not the flattener — both advisories are edges of the verifier's coverage, both currently harmless, both worth the human deciding on before more pages convert in later #402 PRs.

Residual risk:

  • Deep nesting (≥4 spaces). maskFences only recognizes an opening fence at 0–3 leading spaces (CommonMark's own indented-code-block rule), so open.indent is never ≥4 and the dedent check cannot see a fence opened deeper than 3 spaces. Refuted for this PR: #402's 20 nested sites are 18 at 3 spaces + 1 at 2 spaces, all ≤3, so both correct emission and corruption detection work there. Not refuted in general: a future component nested ≥4 spaces whose emission dropped indentation would produce a column-0 stray fence that usually trips the unterminated-fence check — except in the concatenated llms-full.txt, where a later page's fence can pair with it and mask the region as code, evading both checks. The corpus test shares the blind spot (same verifyArtifact). Recorded as advisory #1.
  • False-positive dedent. The predicate flags any less-indented non-blank body line under a 1–3-space opening fence, which is valid CommonMark. No current page or the full build triggers it (corpus + docs build green) and the flattener's own output is always uniformly indented, so it is latent — but an ordinary future edit with a slightly under-indented code block could redden the build. Recorded as advisory #2.
  • llms.txt ↔ default-tab drift. Refuted: the flattener imports renderCommand from the shared translate.mjs, and the flattener emits exactly the pnpm tab asserts the emitted body equals renderCommand(cmd,'pnpm') for every table row, including a multi-segment case. Passthrough segments are identical across tabs by construction (translateSegment returns the normalized segment unchanged for any non-verb first token), so the "passthrough differs across tabs" failure mode cannot occur.

🏄 Chill, clean set here, dudes — the flattener carries its indentation all the way to the beach now, the gate actually wipes out on a bad wave instead of paddling past it, and the baseline's byte-for-byte glassy. Couple of reef edges out past the 4-space break to keep an eye on, but nothing's gonna pitch you today. Send it. 🌊

The dedent check could only ever fire for a fence opened at 1-3 spaces:
`maskFences` recognized an opening fence by CommonMark's top-level rule, so
`open.indent` was never >= 4 and a corrupted emission nested deeper than one
list level was invisible to it. It usually fell through to the
unterminated-fence check — but not in `llms-full.txt`, where a later page's
fence can pair with the stray column-0 one and mask the whole region as code,
which is exactly the artifact the gate exists to protect.

The 0-3 allowance is relative to the enclosing container, not the document, so
seeing a nested fence means knowing the container's column. `maskFences` gains a
`listAware` mode that tracks the innermost list item's content column; the
closing fence is now measured against that column rather than against the
opening fence's own indent, which is what lets a deep fence pair at all while
still pairing a closer that sits below it — the corruption signature.

`verifyArtifact` runs two passes, because its two questions want different
readings. Residual tab JSX keeps the transform's view: widening it there would
mask a region the flattener does rewrite and call a genuine miss clean. Only the
dedent signal is taken from the `listAware` pass — an *unclosed* deep fence
under that reading is usually a plain indented code block quoting a fence, and
reddening a build over one would trade a blind spot for a false positive.

Indentation is now measured in columns with tabs expanded to the next multiple
of four, so a tab-indented fence still reads as the indented code block it is.

Transform behaviour is unchanged by construction: with `listAware` off the
container column is 0, which is the old rule exactly. Verified rather than
assumed — `transform()` output is identical across all 144 shipped pages and
their concatenation, both script versions leave a real build's artifacts
untouched, and `verifyArtifact`'s verdict on every page is unchanged.

77 tests, up from 74. The deep-nesting case fails against the previous commit;
the rest pin the edges the widening must not break.

Copy link
Copy Markdown
Owner Author

Advisory #1 closed in 05766e6. Advisory #2 is a won't-fix, reasoning below.

#1 — the gate was blind past 3 spaces: closed

Confirmed as stated. open.indent could only ever be 1–3, so the dedent signal could not see a fence nested deeper than one list level. Worth being precise about why that mattered, because the review's own "usually trips the unterminated-fence check instead" is the part that doesn't hold in the artifact we actually publish:

1. Outer step:

   - Inner step:

     ```bash        <- 5 spaces: invisible to the check
pnpm add foo         <- corruption
```                  <- stray closer at column 0

# Next page

```ts                <- info string, so not a *closer* ...
const a = 1;
```                  <- ... this pairs with the stray one instead

Fences balance, the whole region masks as code, and verifyArtifact returned []. Not a weaker signal — no signal. That fixture is the new regression test, and it is the one of the four new cases that fails against 84dcbea.

The fix follows the reason the hole existed: CommonMark's 0–3 allowance is relative to the enclosing container, not the document, so seeing a nested fence means knowing the container's column. maskFences gains a listAware mode tracking the innermost list item's content column, and the closing fence is now measured against that column rather than against the opening fence's own indent — which is what lets a deep fence pair at all, while still pairing a closer that sits below it, since that is the corruption signature.

Two deliberate limits, both because a gate that reddens valid builds is worse than one with a blind spot:

  • verifyArtifact runs two passes. Residual tab JSX keeps the transform's view of code. Widening it there would mask a region the flattener does rewrite and call a genuine miss clean — the opposite of the bug being fixed. Only the dedent signal comes from the listAware pass.
  • The unterminated-fence signal stays with the narrow pass. Under the wide reading, an unclosed fence at 4+ spaces is usually just a top-level indented code block quoting a ``` line — valid markdown. My first cut trusted the wide pass with that signal too, and the test I wrote for it failed: a plain indented code block reddened the build. Trading a blind spot for a false positive is not a fix.

Incidental: indentation is now measured in columns with tabs expanded to the next multiple of four, so a tab-indented fence still reads as the indented code block it is. Without that, widening the pattern from ^ {0,3} to ^[ \t]* would have quietly started treating one as a fence at column 1.

#2 — dedent predicate false-positives on flush-left content: won't fix

Correct as described, and I'm leaving it. The two failure modes are the same bytes: "an author wrote a flush-left line inside an indented fence" and "a rewrite dropped the tag's indentation" are indistinguishable to any predicate, because the second is the first — that's what makes the corruption silent in the first place. So there is nothing to narrow on except the thing being detected.

The asymmetry decides it. A false positive is a red build naming the file and the problem, fixed by indenting the block — which the source wanted anyway, since flush-left content inside an indented fence is fragile in CommonMark regardless of this script. A false negative is corrupted llms-full.txt shipped to agents, invisible in the rendered site, which is the entire class of defect this PR exists to close.

It also isn't the build that hits it first: the corpus test runs every shipped page through transform() + verifyArtifact() in pnpm test, so an author who writes that shape sees it locally before CI. Corpus and a full docs build are clean today.

Noting honestly that #1's widening extends #2's surface — the predicate can now also fire on a deep fence — and that this is the trade I'm taking knowingly, not an oversight.

Verification

  • 77 tests, up from 74. Four new: the deep-nesting regression (fails against 84dcbea), a correctly-indented 6-space fence, the indented-code-block non-false-positive, the tab-indent rule, and one pinning that the residual-JSX scan keeps the transform's view of code.
  • Transform behaviour unchanged, measured rather than asserted. With listAware off the container column is 0, which is the old rule exactly — and transform() output is byte-identical across all 144 shipped pages and their concatenation, verifyArtifact's verdict is unchanged on every page, and both script versions run over a real build's artifacts with 0 rewrites and exit 0. The published LLM artifacts still match the pre-change baseline build.
  • pnpm all green; check:conformance 7/7; gen:catalog:check clean; prettier clean.

⚠️ The commit is on claude/pr-418-resolution-ubeqlm, not on this PR's head — it's docs/pm-tabs-flattener-indent plus one commit, so it fast-forwards. Say the word and I'll push it to the PR branch.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment

Preview URL: https://37492006.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

🧹 Nitpick comments (3)
docs/src/components/PackageManagerTabs/translate.mjs (1)

56-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Unknown-manager guard is bypassed for passthrough segments.

translateSegment('cd app', 'cnpm') returns 'cd app' instead of throwing, because the VERBS check precedes the switch. Validating the manager first makes the "programming error, not silent output" contract hold for every segment.

♻️ Validate the manager before the passthrough return
 export function translateSegment(segment, manager) {
+  if (!PACKAGE_MANAGERS.includes(manager)) {
+    throw new Error(`Unknown package manager: ${manager}`);
+  }
   const tokens = segment.split(' ');
   const [verb, ...rest] = tokens;
 
   if (!VERBS.has(verb)) return segment;
   if (manager === 'pnpm') return `pnpm ${segment}`;
🤖 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/src/components/PackageManagerTabs/translate.mjs` around lines 56 - 61,
Update translateSegment to validate the manager before the VERBS passthrough
check, ensuring unknown managers throw for every segment while preserving
existing translation behavior for supported managers.
docs/scripts/flatten-llms-tabs.mjs (1)

70-76: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

A whitespace-only command emits an empty fence.

renderCommand('') yields '', so fenced emits an opening fence, one blank-ish line, and a closing fence — exactly the shape flatten-llms-corpus.test.mjs line 54 reports as "emitted an empty bash fence". An authoring slip (command=" ") therefore fails the build with the corpus message rather than pointing at the tag. Consider leaving the tag untouched (or throwing with the tag text) when there are no segments.

🤖 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/flatten-llms-tabs.mjs` around lines 70 - 76, Update
flattenPackageManagerTabs to detect when the trimmed command passed to
renderCommand is empty before calling fenced. Leave the original
PackageManagerTabs tag unchanged (or raise an error containing the tag text) for
whitespace-only commands, while preserving the existing replacement behavior for
commands with content.
docs/scripts/flatten-llms-corpus.test.mjs (1)

24-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The DOCS_DIR guard can't fire if the directory is missing.

Collection happens at module scope, so a wrong/renamed docs/docs makes readdir reject during import — the file fails to load with an ENOENT instead of the intended "corpus is non-empty" message. Moving the walk inside the guard test (or catching and letting pages stay empty) keeps the diagnostic.

🤖 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/flatten-llms-corpus.test.mjs` around lines 24 - 37, Move the
module-scope corpus collection using walk and DOCS_DIR into the test that
validates pages.length, or catch directory-read failures so pages remains empty.
Ensure a missing or invalid DOCS_DIR reaches the existing assertion and reports
the intended non-empty corpus diagnostic instead of failing during module
import.
🤖 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/flatten-llms-tabs.mjs`:
- Around line 122-131: Update flattenGenericTabs to preserve non-TabItem JSX
found inside a Tabs block when TabItem entries are present, including it in the
generated output alongside the mapped items; alternatively, reject the transform
with a clear diagnostic. Ensure verifyArtifact or the relevant corpus invariant
can detect any content that is not preserved.

---

Nitpick comments:
In `@docs/scripts/flatten-llms-corpus.test.mjs`:
- Around line 24-37: Move the module-scope corpus collection using walk and
DOCS_DIR into the test that validates pages.length, or catch directory-read
failures so pages remains empty. Ensure a missing or invalid DOCS_DIR reaches
the existing assertion and reports the intended non-empty corpus diagnostic
instead of failing during module import.

In `@docs/scripts/flatten-llms-tabs.mjs`:
- Around line 70-76: Update flattenPackageManagerTabs to detect when the trimmed
command passed to renderCommand is empty before calling fenced. Leave the
original PackageManagerTabs tag unchanged (or raise an error containing the tag
text) for whitespace-only commands, while preserving the existing replacement
behavior for commands with content.

In `@docs/src/components/PackageManagerTabs/translate.mjs`:
- Around line 56-61: Update translateSegment to validate the manager before the
VERBS passthrough check, ensuring unknown managers throw for every segment while
preserving existing translation behavior for supported managers.
🪄 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: 1fdd3ace-1bc1-4ca8-9597-e70d4cab110a

📥 Commits

Reviewing files that changed from the base of the PR and between 92e94e0 and 05766e6.

📒 Files selected for processing (6)
  • docs/scripts/flatten-llms-corpus.test.mjs
  • docs/scripts/flatten-llms-gate.test.mjs
  • docs/scripts/flatten-llms-tabs.indent.test.mjs
  • docs/scripts/flatten-llms-tabs.mjs
  • docs/scripts/package-manager-translate.test.mjs
  • docs/src/components/PackageManagerTabs/translate.mjs

Comment on lines +122 to +131
return (
lead +
items
.map(([, attrs, body]) => {
const label = attrLabel(attrs);
const text = reindent(body.trim());
return label ? `${indent}${TAB_HEADING} ${label}\n\n${text}` : text;
})
.join('\n\n')
);

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find <Tabs> blocks whose inner content includes non-TabItem lines.
fd -e md -e mdx . docs/docs | while read -r f; do
  python3 - "$f" <<'PY'
import re, sys
src = open(sys.argv[1], encoding='utf8').read()
for m in re.finditer(r'<Tabs(?=[\s>])[^>]*>([\s\S]*?)</Tabs>', src):
    inner = re.sub(r'<TabItem\b[^>]*>[\s\S]*?</TabItem>', '', m.group(1))
    if inner.strip():
        print(sys.argv[1], repr(inner.strip()[:120]))
PY
done

Repository: allxsmith/bestax

Length of output: 3276


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## candidate file"
fd -a 'flatten-llms-tabs\.mjs$' . | sed 's#^\./##'

echo "## outline"
ast-grep outline docs/scripts/flatten-llms-tabs.mjs || true

echo "## relevant ranges"
sed -n '1,220p' docs/scripts/flatten-llms-tabs.mjs | cat -n

echo "## parse behavior probe (same regex/branch logic as source)"
python3 - <<'PY'
import re, ast, pathlib

src = pathlib.Path('docs/scripts/flatten-llms-tabs.mjs').read_text()
print('contains TabItem-specific emit around lines 122-131:', 'TabItem' in src and 'label' in src and 'attrLabel' in src)

# Determine whether the file parses as valid JS; use babel/meriyah if available, otherwise source read is enough evidence for regex behavior.
tab_inner = '<Tabs command="x">\n<TabItem label="\n  first" />\n\n  admonition\n\n  commentary\n\n<TabItem label="\n  second" />\n</Tabs>'
# The script logic in that range only maps matched TabItems; we can re-map that behavior.
tab_items = re.findall(r'<\s*TabItem\b([^>]*)>([\s\S]*?)<\s*/\s*TabItem\s*>', tab_inner)
result = ''.join(f'{i}: {attrs!r} body={body!r}' for i, (attrs, body) in enumerate(tab_items))
non_tab = re.sub(r'<\s*TabItem\b[^>]*>([\s\S]*?)<\s*/\s*TabItem\s*>', '', tab_inner)
print('extracted TabItem bodies:', result)
print('non-TabItem inner remainder:', repr(non_tab))
print('non-TabItem remainder contains "admonition":', 'admonition' in non_tab)
print('non-TabItem remainder contains "commentary":', 'commentary' in non_tab)
PY

Repository: allxsmith/bestax

Length of output: 12135


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## verifyArtifact"
sed -n '361,396p' docs/scripts/flatten-llms-tabs.mjs | cat -n

echo "## lint/test/format scripts"
sed -n '1,220p' package.json 2>/dev/null | cat -n || fd -a 'package\.json$' . | while read -r f; do echo "--- $f"; sed -n '1,220p' "$f" | cat -n; done

echo "## changed status"
git diff --stat HEAD || true
git diff -- docs/scripts/flatten-llms-tabs.mjs HEAD | sed -n '1,220p' || true

Repository: allxsmith/bestax

Length of output: 4072


Preserve and validate non-<TabItem> content inside <Tabs>.

flattenGenericTabs drops all non-<TabItem> JSX under <Tabs> when at least one TabItem is found, so stray content like embedded admonitions can disappear from the generated LLM artifact without failing verifyArtifact or a corpus invariant. Emit that content too, or fail the transform with a clear diagnostic.

🤖 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/flatten-llms-tabs.mjs` around lines 122 - 131, Update
flattenGenericTabs to preserve non-TabItem JSX found inside a Tabs block when
TabItem entries are present, including it in the generated output alongside the
mapped items; alternatively, reject the transform with a clear diagnostic.
Ensure verifyArtifact or the relevant corpus invariant can detect any content
that is not preserved.

@allxsmith
allxsmith merged commit 0d8bfa5 into main Jul 30, 2026
22 checks passed
@allxsmith
allxsmith deleted the docs/pm-tabs-flattener-indent branch July 30, 2026 11:59
@bestax-release-bot

Copy link
Copy Markdown

🎉 This PR is included in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@bestax-release-bot

Copy link
Copy Markdown

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@bestax-release-bot

Copy link
Copy Markdown

🎉 This PR is included in version 5.8.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@bestax-release-bot

Copy link
Copy Markdown

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants