fix(docs): stop comment examples from triggering Fern's rolldown bundling - #12388
Merged
Conversation
…ling Fern's `experimental.mdx-components` bundler regex-scans every .js/.jsx/.ts/.tsx file under docs/fern/components for import specifiers. Anything neither relative nor in its allowlist (react, react-dom, @mdx-js/react, next) is treated as a third-party dependency, and Fern runs `npx --quiet --yes rolldown@1.1.4` once per offending file — a registry download on every docs build. The scan does not skip comments. Three docblock usage examples matched it, so every docs build fetched rolldown three times over the network. That is the direct cause of the intermittent preview failures on docs PRs: rolldown exited with code 127 / sh: 1: rolldown: not found rolldown exited with code 1 / ERR_MODULE_NOT_FOUND ... _npx/.../rolldown/ No component actually depends on anything outside Fern's allowlist. TerminalDemo deliberately loads asciinema-player from jsDelivr at runtime to avoid a bundled import; its only real import is react. So the fetch was pure waste as well as a flake. Quotes the specifiers in those examples with backticks instead, and says why in the surrounding comment so the next author does not revert it. Adds docs/fern/scripts/check_component_imports.py, which mirrors fern-api's own regex and allowlist, wired as a pre-commit hook over docs/fern/components. Verified: the check exits 1 on the three files as they stand on main and 0 after; an independent JS replica of Fern's detector agrees; `fern check` reports 0 errors and 1292 warnings both before and after, so the baseline is unchanged. Signed-off-by: Dan Gil <dagil@nvidia.com>
github-actions
Bot
added
documentation
dagil-nvidia
added a commit
that referenced
this pull request
Jul 29, 2026
Both files copied the RecipeStyles docblock convention, including its usage example. Fern's mdx-components bundler regex-scans component files for imports without skipping comments, so each quoted `@/components/...` specifier makes it shell out to `npx rolldown` — a per-build registry fetch that flakes docs previews. Adding these two would have taken the build from three such fetches to five. Quotes the specifiers with backticks, matching the treatment #12388 applies to RecipeStyles, ReferenceStyles and TerminalDemo on main. Comments only. Verified with a JS replica of Fern's detector (regex and allowlist extracted from the fern-api@5.80.2 tarball): neither new file is flagged. The three remaining hits on this branch come from main and are fixed by #12388. Signed-off-by: Dan Gil <dagil@nvidia.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
WalkthroughA new Python scanner checks Fern component imports for non-allowlisted package specifiers. A pre-commit hook runs it, and component documentation comments are updated to describe the scanner’s backtick formatting requirement and runtime loading behavior. ChangesFern import validation
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/fern/scripts/check_component_imports.py (1)
45-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a focused regression test for the scanner regex.
The regex and
offenders()logic are the core contract: quoted non-relative imports in comments must be reported, while backtick examples, relative imports, and allowlisted packages must pass. Add a small unit or CLI test covering these cases.As per coding guidelines, and as required by
.ai/python-guidelines.md, critical regex behavior should have a matching test.🤖 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/fern/scripts/check_component_imports.py` around lines 45 - 75, Add a focused regression test for SPECIFIER and offenders() covering quoted non-relative imports in comments as reported offenders, while excluding backtick examples, relative imports, and ALLOWLIST packages. Keep the test small and exercise the scanner’s existing public behavior through offenders().Sources: Coding guidelines, Path instructions
🤖 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.
Nitpick comments:
In `@docs/fern/scripts/check_component_imports.py`:
- Around line 45-75: Add a focused regression test for SPECIFIER and offenders()
covering quoted non-relative imports in comments as reported offenders, while
excluding backtick examples, relative imports, and ALLOWLIST packages. Keep the
test small and exercise the scanner’s existing public behavior through
offenders().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b4264591-850a-42f9-9e72-12c13deb11dc
📒 Files selected for processing (5)
.pre-commit-config.yamldocs/fern/components/RecipeStyles.tsxdocs/fern/components/ReferenceStyles.tsxdocs/fern/components/TerminalDemo.tsxdocs/fern/scripts/check_component_imports.py
Contributor
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d7bddda8a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![dynamo-review-agent[bot]](https://avatars.githubusercontent.com/u/201626793?s=60&v=4){kind=small}
The SPECIFIER regex and ALLOWLIST are copied from fern-api's bundler and are
the whole contract of this check, so per .ai/python-guidelines.md ("when
changing a critical regex, add a one-line test to prove it matches") they get
covered directly.
Adds a --test mode following the convert_callouts.py precedent for
docs/fern/ scripts, with eleven cases through offenders(): the two the check
exists for (a quoted specifier in a comment is caught, a backticked one is
not), both allowlist shapes, bare and scoped non-allowlisted packages,
relative and parent-relative paths, re-export, dynamic import(), require(),
and dedup.
convert_callouts.py --test is referenced nowhere in .github/ or
.pre-commit-config.yaml, so it only runs when someone remembers. To avoid
adding a second test that rots, wires --test as its own pre-commit hook
scoped to the script itself: it fires exactly when the regex or allowlist is
edited and costs nothing on any other commit.
Verified: 11/11 pass. Mutation-checked against two independent breaks — a
scanner that skips comment lines (the exact bug this PR fixes) and a
shortened allowlist — and the suite fails on each, so it is not vacuous.
Addresses the CodeRabbit review on #12388.
Signed-off-by: Dan Gil <dagil@nvidia.com>
`fern check` validates configuration and navigation, never component syntax, so a component that does not compile merges green and only surfaces when Fern builds the site. #12330 shipped a LandingStyles.tsx whose CSS comment carried raw backticks that closed the enclosing template literal; every gate passed and it took #12402 to repair main. esbuild transforms each file without bundling, which is exactly the missing syntax gate. It runs in the existing fern-check job, which already sets up Node 22 and is already scoped to docs changes, so this adds one step rather than a job. The version is pinned so a bad esbuild release cannot silently redefine the gate, and an empty file list is a hard error rather than a vacuous pass. Verified by reproducing the #12330 defect class: injecting a single raw backtick into a CSS comment in RecipeStyles.tsx makes the step exit 1 with 'Expected ";" but found "node"', and the unmodified tree exits 0. Signed-off-by: Dan Gil <dagil@nvidia.com>
…s, SPDX Three review findings on this branch. The scanner's regex is copied verbatim from fern-api, but Python's \w is Unicode-aware and JavaScript's is ASCII-only, so the leading (?:^|[^\w.]) guard diverged: a Unicode letter sitting directly against the import keyword is a word character in Python and not in JavaScript. Fern bundles that file; the check stayed silent. re.ASCII restores the intended verbatim behaviour, pinned by a case that fails without it. The esbuild step parsed only .ts and .tsx while the scanner and its pre-commit pattern both cover .js and .jsx, so a broken .js helper would have slipped the syntax gate. Both lists now name the same four suffixes Fern itself scans, through one shell function so they cannot drift again. docs/fern/AGENTS.md requires the copyright range 2025-2026; the new script carried 2026 alone. Verified: the self-test is 12/12, and 11/12 with re.ASCII removed, so the new case is not vacuous. The esbuild step exits 0 on this tree and on main's components, and exits 1 when a .js file with a syntax error is added — the suffix that was previously unguarded. Signed-off-by: Dan Gil <dagil@nvidia.com>
Follow-up on the remaining review findings. SPECIFIER and ALLOWLIST are transcribed from one fern-api release, and until now nothing enforced that. A Fern bump would leave the check quietly vouching for a bundler it no longer describes — stale but green, the worst failure mode for a gate. DERIVED_FROM_FERN records the release the constants came from and the scan refuses to report a clean tree when fern.config.json has moved past it, naming the drift and pointing at the re-derivation recipe now in the docstring. --test keeps working across a bump, since re-deriving the constants is exactly when the cases need to run. The repo root came from COMPONENTS.parents[2], which silently produced a wrong path if the script ever moved out of docs/fern/scripts/. It is now derived from FERN_ROOT alongside the other paths, and a missing fern.config.json is reported as "the script has moved" rather than surfacing as a confusing relative_to failure. Argument handling accepted anything containing --test and ignored the rest. It now takes [] or [--test] exactly and exits 2 with a usage line otherwise. Verified: clean tree exits 0 and --test is 12/12; a simulated bump to 5.83.1, a malformed fern.config.json, and a deleted one each exit 1 with a distinct message; an unknown flag and a trailing argument after --test each exit 2; an injected offender still reports the repo-relative path docs/fern/components/Offender.tsx. Signed-off-by: Dan Gil <dagil@nvidia.com>
The backtick convention this branch introduced was the wrong shape. It made every docblock example look like broken code, it had to be explained three times, and it left the trap one careless edit away — an author who writes the example with ordinary quotes reintroduces the flaky build. Fern's scan only covers .js, .jsx, .ts and .tsx. Markdown is invisible to it, so components/README.md can carry the examples with real quotes, keep them copy-pasteable, and make the failure structurally impossible rather than conventionally avoided. The three docblocks now point at it and say why. The README also collects the component conventions that were previously scattered or unwritten: no third-party dependencies, when a style block needs dangerouslySetInnerHTML, and no backticks inside a CSS template literal. The scanner keeps Fern's exact pattern. Replacing it with a looser over-catching one, which the README move seemed to permit, flags ordinary value exports: `export const KIND = "all"` in InstallSelector.tsx, install-selector-data.ts and releases.data.ts all trip it. That is why the real pattern insists on `from` or a directly quoted specifier, and three cases now pin it. Also stops REPO_ROOT raising IndexError at import time when the script runs from a shallow path, which masked the check_derivation message explaining the move. Verified: self-test 16/16; the three value-export cases fail against the looser pattern, so they are not vacuous; a comment example is still caught; an injected offender still reports docs/fern/components/Offender.tsx; running from a shallow path now exits 1 with the moved-script message instead of a traceback; the esbuild step exits 0; fern check reports 0 errors with README.md present in components/. Signed-off-by: Dan Gil <dagil@nvidia.com>
The hook's SPECIFIER and ALLOWLIST are transcribed from a specific fern-api release (DERIVED_FROM_FERN in check_component_imports.py), and its check_derivation() gate refuses to vouch for a clean tree once docs/fern/fern.config.json moves past that version. Without fern.config.json in the hook's files regex, a Fern version bump lands without firing the hook, so the mirrored constants stay silently stale and the rolldown-bundling regression can return with no local signal. Add docs/fern/fern.config.json to the hook's trigger regex so a version bump forces the hook to run and check_derivation() blocks the commit until DERIVED_FROM_FERN (and, when the upstream regex or allowlist changed, SPECIFIER and ALLOWLIST) are re-derived from the new release. Validation: - python3 docs/fern/scripts/check_component_imports.py --test -> 16/16 - python3 docs/fern/scripts/check_component_imports.py -> exit 0 - Regex match check: 'docs/fern/fern.config.json' now matches; unrelated paths (README.md, fern.config.jsonx) still do not. - pre-commit run check-component-imports --files docs/fern/fern.config.json now selects the file and passes on this tree. Signed-off-by: Dan Gil <dagil@nvidia.com>
This comment has been minimized.
This comment has been minimized.
Collaborator
Author
|
/ok to test 050f7c8 |
…e scanner docs/fern/documentation-style-guide.md requires the full Apache-2.0 block on code files (.py, .sh, .yaml, Dockerfile), not just the two SPDX lines. The new scanner carried only the SPDX pair; the sibling sync_site_css.py is the same way, but an existing gap is not licence to add another. Resolving REPO_ROOT through a slice of Path.parents also crashed the hook on Python 3.9: PurePath.parents only became sliceable in 3.10. The hook is language: system, so it runs under whichever python3 is on PATH, and a gate whose whole purpose is to fail loudly instead died in a TypeError before reaching any check. .parent.parent is equivalent for every path at or below the repo root, never raises, and needs no minimum version. Verified: pre-commit run --files on this script is green on both hooks under the 3.9 interpreter pre-commit resolves, where it previously raised TypeError twice; the self-test is 16/16 and the tree scan exits 0. Signed-off-by: Dan Gil <dagil@nvidia.com>
Collaborator
Author
|
/ok to test 2bc794c |
dagil-nvidia
added a commit
that referenced
this pull request
Aug 2, 2026
CodeRabbit was right that the SGLang multimodal note read as if EPD, E/PD and E/P/D were KV-aware routing modes. They are multimodal-serving disaggregation patterns. Split the two claims: KV-aware routing applies to aggregated workers, disaggregation is stated separately. releases.json regenerates from the same source. Also removes the import-shaped examples from the ReferenceStyles, RecipeStyles and TerminalDemo docblocks. Fern's bundler scans component files for import specifiers without skipping comments; a non-relative "@/..." specifier sends it to `npx rolldown`, which has no network on the runner. That is what failed this PR's `Preview or publish docs` job: Failed to bundle third-party imports in .../RecipeStyles.tsx: rolldown exited with code 127 -- sh: 1: rolldown: not found Publishes on main are unaffected -- only the preview path bundles components -- so this broke previews on every docs PR while the site kept updating. #12388 fixes the same three files repo-wide and adds a guard; this is the minimum to unblock here and will conflict trivially with it. gen_llms_tables --check 0 stale; fern check 0 errors, 1292 warnings; 14 tests pass; all three components parse under esbuild. Signed-off-by: Dan Gil <dagil@nvidia.com>
Signed-off-by: Dan Gil <dagil@nvidia.com> # Conflicts: # .pre-commit-config.yaml # docs/fern/components/TerminalDemo.tsx
Collaborator
Author
|
/ok to test be40389 |
dagil-nvidia
added a commit
that referenced
this pull request
Aug 7, 2026
#12241 and this PR both rewrote compatibility.mdx and gen_llms_tables.py and conflicted in both. Landing them in sequence would have meant resolving that conflict blind on the second rebase; folding resolves it once with both sides in hand. #12241 is closed in favour of this. Carried over from #12241: * The driver-floor table, which inverts the support matrix -- read across from a driver you already have to the newest release each backend runs on it. <ReleaseSupportMatrix /> renders only the forward view (release -> min driver), so this stays a generated span, now placed beside the component it inverts. * The SGLang multimodal and router prose corrections, the v1.3.0 release note and release-history fixes, and the UpgradePanel change. * test_gen_llms_tables.py (14 tests) with its pytest.ini and pre-commit hook. * The CUDA compatibility link, now pointing at /latest/ so it resolves without a redirect hop. Dropped from #12241, superseded here: * The generated support-matrix accordion and its render_support_matrix renderer. <ReleaseSupportMatrix /> replaces that rendering, and the data still reaches agents through the llms-tables twin. * The raw feature-interaction markdown tables, replaced by <FeatureInteractions />. Their per-cell notes live in releases.data.ts. Corrects a factual error this fold exposed. FEATURE_INTERACTIONS marked the SGLang Multimodal x KV-Aware Routing cell "no" ("This feature combination is not supported"), while the merged feature table on the same page described it as supported with a fallback -- the page contradicted itself. multimodal-kv- routing.md is the authority and says supported: hash forwarding is upstream in SGLang 0.5.13+ and Dynamo pins 0.5.16. The cell is now "yes", noting that a custom build without the patch degrades to text-prefix routing. Not carried over: the RecipeStyles, ReferenceStyles and TerminalDemo comment fixes, which are #12388's and stay there. Those three files are byte-identical to main on this branch. Validation: gen_llms_tables.py --check clean; check_agent_twins.py clean on all three governed pages with its self-test at 14/14; pytest test_gen_llms_tables.py 14 passed; ruff clean. Verified the corrected note reaches the agent twin. Signed-off-by: Dan Gil <dagil@nvidia.com>
Fixes the failing pre-commit check. The check-published-styles-selftest hook was invoking check_published_styles.py with the filename appended, and that script's argparse takes no positional arguments: check_published_styles.py: error: unrecognized arguments: docs/fern/scripts/check_published_styles.py main had already fixed this by adding pass_filenames: false to the hook. This branch is 52 commits behind and predates that fix, so it kept running the broken form. The merge alone did not bring it. This branch inserts check-component-imports directly after that hook, so the auto-merge resolved the hunk to the branch side and dropped main's added line silently. Restored explicitly; the hook block is now byte-identical to main's. Validation: the published-styles self-test passes 7/7, the component-import scanner reports no unbundleable specifiers, and its self-test passes 16/16. Reproduced the original failure by invoking the script with a filename argument, and confirmed pass_filenames: false is what prevents it. Signed-off-by: Dan Gil <dagil@nvidia.com>
Collaborator
Author
|
/ok to test 3629cdf |
dmitry-tokarev-nv
left a comment
dmitry-tokarev-nv
left a comment
Contributor
There was a problem hiding this comment.
Review of the new component-import gate — six blocking findings inline, each reproduced against head 3629cdf.
The direction is right: fern check genuinely does not validate component syntax, and transcribing Fern's scanner is a reasonable way to close that. The findings are about the gate's own failure modes — two ways it silently passes files Fern will reject, one way it reds the entire repo, and one crash path.
Non-blocking items I left out of the inline comments, happy to file separately: offenders() flags quoted filenames in ordinary prose (' * We import the styles from "main.css".' -> ['main.css']), and the one CASES entry meant to pin prose safety is quote-free so it can never catch that; --test exercises only offenders(), leaving main(), check_derivation() and file discovery uncovered; {js,jsx,ts,tsx} and components are hand-copied across three files while docs.yml's experimental.mdx-components (the actual source of truth) is read by none of them; LandingStyles.tsx and BlogStyles.tsx still carry backtick-quoted docblock examples pointing at a RecipeStyles.tsx passage this diff replaced; and a few README specifics (welcome.mdx / community/README.mdx do not exist; the "no quoted non-relative specifier" rule is contradicted by the react allowlist).
dagil-nvidia
enabled auto-merge (squash)
August 9, 2026 01:09
All six blocking findings from @dmitry-tokarev-nv. Each was reproduced before and after, against fern-api 5.92.0 where his greps were run. Unicode whitespace defeated the scanner. re.ASCII is not selective: it narrowed \s as well as \w, and JavaScript's \s is Unicode-aware. An import separated by a non-breaking space matched in Fern and not here, so the preview failed with the ERR_MODULE_NOT_FOUND this gate exists to prevent while the gate stayed green. Both character classes now spell out ASCII explicitly and the flag is gone. Checking his list turned up one more: Python's \s omits U+FEFF, which ECMA-262 includes, so that is matched by hand. Verified across U+0020, 00A0, 2009, 2028, 3000 and FEFF; the old pattern missed every one but the first. The scanner failed open on a missing directory. rglob on a renamed components/ yields nothing and raises nothing, so it reported a clean tree it never opened -- exit 0 under his repro. It now fails closed, and separately fails when the directory exists but holds no component sources, since docs.yml still points Fern at it either way. A count of files scanned is printed so a silent pass is visible. Locale-dependent decoding. read_text() followed the locale while Fern always decodes UTF-8. Under LC_ALL=C it raised a pathlib traceback instead of the gate's message; under a latin-1 locale it scanned mojibake silently. Worse at the version read, where UnicodeDecodeError subclasses ValueError and was caught and misreported as "could not read a version" on a good file. Both reads now pin encoding="utf-8", matching four sibling scripts. The re-derivation recipe returned nothing. It grepped minifier-assigned symbol names, which churn every release: the allowlist binding was TZu at 5.80.2 and is Vtl at 5.92.0, and SGm is gone. A recipe that returns nothing invites bumping DERIVED_FROM_FERN without re-deriving, converting the gate into a silent no-op. The recipe is now value-anchored, and each command was run verbatim as a reader would copy it. Worth recording: at 5.92.0, twelve minor releases on, the allowlist, the suffix list and the regex are all unchanged, and the regex is byte-identical to SPECIFIER. A swallowed failure in the workflow. --outdir="$(mktemp -d)" is a word on the xargs line, so set -e never saw the substitution's status; a failing mktemp expanded to --outdir= and esbuild ran anyway. Hoisted to its own assignment, which reproduces as fatal. --outbase=. added so a future Foo.ts plus Foo.tsx cannot abort the run with a path collision unrelated to component syntax. The step also rode the repo-wide docs filter, which matches 458 of the last 1172 commits on main while only 21 touched a component source. A narrow fern_components filter now gates it, wired through the changed-files action and documented in FILTERS.md. The canonical example shipped the anti-pattern. components/README.md is now the declared source of truth for usage, and its TerminalDemo example used a site-absolute src -- the exact form check_asset_paths.py rejects -- naming a .cast file that does not exist. That gate could not see it, because DEFAULT_GLOBS covered components/**/*.tsx and *.ts but not *.md. The example now uses the relative form with the real asset, components/**/*.md is in DEFAULT_GLOBS, and the relative-path constraint is restored to TerminalDemo's src TSDoc, which this PR had removed as the only statement of it in the repo. Validation: the import scanner reports 33 sources clean, its self-test passes 16/16, the asset gate covers 392 files clean, and ruff is clean. Both workflow YAML files parse. His three script reproductions were re-run: the renamed directory now exits 1, LC_ALL=C no longer traces back, and all three greps return their values against 5.92.0. Not addressed here, and worth a follow-up rather than a silent omission: the esbuild step still fetches 4.35 MB via npx on every run with no lockfile, integrity pin, or cache, so a registry 5xx can red a docs PR. Narrowing the filter cuts how often that exposure is taken but does not remove it. Signed-off-by: Dan Gil <dagil@nvidia.com>
Collaborator
Author
|
@dmitry-tokarev-nv all six are addressed in Unicode whitespace. You were right that Your list turned up one more. Python's Fails open. Reproduced exactly: renaming Locale decoding. Both reads pin The re-derivation recipe. Confirmed broken: Worth recording from that exercise: at 5.92.0, twelve minor releases past the pin, the allowlist, the suffix list and the regex are all unchanged, and the regex is byte-identical to The workflow. On the filter: the step now rides a narrow The README example. All three of your points were correct, including that One I did not fix, flagging rather than omitting: the esbuild step still fetches 4.35 MB via Validation: import scanner 33 sources clean, self-test 16/16, asset gate 392 files clean, ruff clean, both workflow YAML files parse. |
Collaborator
Author
|
/ok to test 04391df |
The component-parsing step ran `npx --yes esbuild@0.24.0`, which re-fetches 4.35 MB from the registry on every run with no lockfile, no integrity check, and no cache. The step is a hard dependency of pre-merge-status-check, so a registry 5xx would red a PR that only touched an .mdx page. Now installed with --no-save and invoked from node_modules/.bin, with the npm store cached and keyed on the pinned version, so a warm run needs no network and a version bump misses the cache deliberately rather than serving a stale tarball for a new pin. node_modules/ and package-lock.json are already ignored at any depth, so the tree stays clean. Completes the last item from @dmitry-tokarev-nv's review of the workflow step. Signed-off-by: Dan Gil <dagil@nvidia.com>
Collaborator
Author
|
/ok to test fcb7e8d |
Collaborator
Author
|
@dmitry-tokarev-nv the esbuild is now installed with That closes all six of your findings plus the reliability concern. Ready for another look. |
dmitry-tokarev-nv
approved these changes
Aug 10, 2026
dmitry-tokarev-nv
left a comment
dmitry-tokarev-nv
left a comment
Contributor
There was a problem hiding this comment.
Thank you for addressing comments. LGTM
dagil-nvidia
added a commit
that referenced
this pull request
Aug 10, 2026
#12388 merged and moved main, conflicting on .pre-commit-config.yaml. Both sides append to the same hook list: this branch adds gen-llms-tables and gen-llms-tables-check, main adds check-component-imports and its self-test. Different hooks, no overlap, so the resolution is the union. Taking either side alone would have silently dropped a gate nobody would notice missing. Resolving surfaced a real defect in this branch's own hook. gen-llms-tables-check was missing 'pass_filenames: false', so pre-commit appended the matched filenames to the command and gen_llms_tables.py --check rejected them as unrecognized arguments. The hook therefore failed on any commit touching releases.data.ts or its outputs -- which is precisely when it is supposed to run, so the gate was inert exactly when it mattered. Its sibling gen-llms-tables already sets the flag; this one did not. Running --check by hand passes because no filenames are appended, which is why it went unnoticed. Everything else auto-merged, including releases.data.ts and the artifacts generated from it. An auto-merged generated file is not trustworthy merely because git emitted no marker, so freshness was verified rather than assumed. Validation: full pre-commit run passes; 32 hooks parse with all four present and no conflict markers; gen_llms_tables --check confirms releases.json and every generated span still match the merged source. Signed-off-by: Dan Gil <dagil@nvidia.com>
dagil-nvidia
added a commit
that referenced
this pull request
Aug 10, 2026
#12388 merged and moved main 8 commits, conflicting on .github/workflows/pre-merge.yml. Both sides add an output to the changed-files job: this branch adds api_docs, main adds fern_components. Different outputs for different filters, so the resolution is the union -- dropping either would leave a job keyed on an output that no longer exists. The merge also brought Python source changes that alter docstrings the reference renders, so gen_python_api.py --check went stale on content this branch never touched. Regenerated; common.mdx was the only page affected. That is the case this PR's own docs describe: a branch's generated output goes stale on unrelated main churn, and the freshness gate attributes it to the branch, so the regeneration has to happen at merge time rather than earlier in review. Validation: both changed-files outputs present and each defined in filters.yaml; workflow YAML parses with 6 outputs; all three generators --check clean; 205 tests pass; fern check 0 errors. Signed-off-by: Dan Gil <dagil@nvidia.com>
dagil-nvidia
added a commit
that referenced
this pull request
Aug 10, 2026
#12388 merged and moved main, conflicting on .github/filters.yaml and .github/FILTERS.md. filters.yaml is a union: this branch rewrites the comment above the docs filter to record why the repo-wide catch-alls moved to `ignore`, main adds a new fern_components filter. Different concerns, both kept. FILTERS.md was not a union -- the two sides contradict on what `docs` gates. This branch says four jobs; main says "Nothing (classification only)". Checked against the workflow rather than picking a side: in the merged pre-merge.yml, outputs.docs gates Docs Lint, Fern Configuration Check, Docs Website Composition Check, and Fern Broken Links Check. Four jobs, so this branch's row is right. Main's row is already wrong on main, where its own workflow keys three jobs on docs while the table claims none. Taking this branch's row plus main's new fern_components row fixes that drift as a side effect, and is the same defect Dmitry flagged earlier on this PR -- a hand-maintained table drifting from the workflow it describes. Validation: both files parse; filters.yaml carries 26 keys with docs, fern_components and ignore all present; docs_lint --scan docs exits 0. Signed-off-by: Dan Gil <dagil@nvidia.com>
1 task
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
3 participants
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
Docs previews fail intermittently on PRs that touch
docs/**, with two variants of the same error:Both are the same root cause, and it is not an infrastructure problem. A comment is being read as an import.
Root cause
fern-api@5.80.2— the version pinned indocs/fern/fern.config.json— scans every.js/.jsx/.ts/.tsxfile under theexperimental.mdx-componentsdirectory with this regex:Any specifier that is neither relative (
./,../) nor in its allowlist —["react","react-dom","@mdx-js/react","next"]— is treated as a third-party dependency, and Fern shells out to:once per offending file. That is a registry download on every docs build. When the fetch fails outright you get exit 127; when it leaves a half-written npx cache you get
ERR_MODULE_NOT_FOUND.The scan does not strip comments. Three docblock usage examples matched it:
docs/fern/components/RecipeStyles.tsximport { RecipeStyles } from "@/components/RecipeStyles";docs/fern/components/ReferenceStyles.tsximport { ReferenceStyles } from "@/components/ReferenceStyles";docs/fern/components/TerminalDemo.tsx`import "asciinema-player"`and the@/components/TerminalDemoexampleNo component actually depends on anything outside Fern's allowlist.
TerminalDemodeliberately loads asciinema-player from jsDelivr at runtime (line 58) precisely to avoid a bundled import; its only real import isreact. So all three rolldown fetches were pure waste on top of being a flake.What changed
docs/fern/scripts/check_component_imports.py, which mirrorsfern-api's own regex and allowlist verbatim, wired as a pre-commit hook scoped todocs/fern/components/**. It runs locally for authors and in the existingpre-commitpre-merge job.No behavioral change to any component. The diff to the three
.tsxfiles is comments only.Verification
check_component_imports.pyexits 1 naming all three files againstmainas it stands, and 0 after the change — red baseline before green.fern-api@5.80.2tarball, agrees: three hits before, zero after.fern check: 0 errors, 1292 warnings both before and after — baseline unchanged.pre-commit run --files <changed>: all applicable hooks pass, including the new one.Follow-up
#12330 adds
BlogStyles.tsxandLandingStyles.tsxfollowing the sameRecipeStylesdocblock convention, so it inherits the trap and would take the build from three rolldown fetches to five. Once this lands, that branch needs the same one-line treatment; the new pre-commit hook will catch it.If a component ever needs a genuine third-party dependency, this check has to be revisited alongside a
package.jsonandnode_modulesfor the docs project — which is exactly what Fern's error message asks for. That is documented in the script's docstring.The upstream bug is that Fern's detector does not skip comments. Worth reporting separately.
Update: merged main, and the pre-commit failure is fixed
The failing
pre-commitcheck was not this PR's code. Thecheck-published-styles-selftesthook was invoking the script with thefilename appended, and that script's argparse takes no positional arguments:
mainhad already fixed this by addingpass_filenames: falseto the hook.This branch was 52 commits behind and predated that fix, so it kept running
the broken form.
Merging
mainalone did not bring it. This branch insertscheck-component-importsdirectly after that hook, so the auto-merge resolvedthe hunk to the branch side and dropped
main's added line silently. Restoredexplicitly; the hook block is now byte-identical to
main's.Validation: the published-styles self-test passes 7/7, the component-import
scanner reports no unbundleable specifiers, and its self-test passes 16/16.
The original failure was reproduced by invoking the script with a filename
argument, confirming
pass_filenames: falseis what prevents it.Summary by CodeRabbit
Documentation
Chores