fix(docs): use MDX comments in recipe-health.md so Fern publish parses#1365
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR updates the comment syntax used for AICR-HEALTH section delimiters and the license header in Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/user/recipe-health.md (1)
34-82:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate marker-contract consumers to match Line 34 and Line 82.
The new
{/* ... */}markers here are correct for Fern/MDX, buttools/health/main_test.gostill hard-codes<!-- BEGIN/END AICR-HEALTH -->; that guard test will fail until updated. Please also aligndocs/design/009-recipe-health-tracking.mdso contributor-facing docs describe the current marker contract.Proposed follow-up patch
diff --git a/tools/health/main_test.go b/tools/health/main_test.go @@ - for _, marker := range []string{"<!-- BEGIN AICR-HEALTH -->", "<!-- END AICR-HEALTH -->"} { + for _, marker := range []string{"{/* BEGIN AICR-HEALTH */}", "{/* END AICR-HEALTH */}"} { if !strings.Contains(string(data), marker) { t.Errorf("%s is missing splice marker %q", docPath, marker) } } diff --git a/docs/design/009-recipe-health-tracking.md b/docs/design/009-recipe-health-tracking.md @@ -...between `<!-- BEGIN AICR-HEALTH -->` / `<!-- END AICR-HEALTH -->`. +...between `{/* BEGIN AICR-HEALTH */}` / `{/* END AICR-HEALTH */}`.🤖 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/user/recipe-health.md` around lines 34 - 82, The markers in the recipe-health.md file have been updated from HTML comments (<!-- BEGIN/END AICR-HEALTH -->) to JSX/MDX comments ({/* BEGIN AICR-HEALTH */}), but this change needs to be reflected in the marker-contract consumers. Update tools/health/main_test.go to search for the new JSX/MDX marker format instead of the old HTML comment syntax to ensure the guard test passes with the updated markers. Also update docs/design/009-recipe-health-tracking.md to document the new marker format so that contributor-facing documentation accurately describes the current marker contract.
🤖 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.
Outside diff comments:
In `@docs/user/recipe-health.md`:
- Around line 34-82: The markers in the recipe-health.md file have been updated
from HTML comments (<!-- BEGIN/END AICR-HEALTH -->) to JSX/MDX comments ({/*
BEGIN AICR-HEALTH */}), but this change needs to be reflected in the
marker-contract consumers. Update tools/health/main_test.go to search for the
new JSX/MDX marker format instead of the old HTML comment syntax to ensure the
guard test passes with the updated markers. Also update
docs/design/009-recipe-health-tracking.md to document the new marker format so
that contributor-facing documentation accurately describes the current marker
contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 153c3dcf-326b-48ee-8ba7-28f9e3b36b8e
📒 Files selected for processing (2)
Makefiledocs/user/recipe-health.md
Coverage Report ✅
Coverage BadgeNo Go source files changed in this PR. |
Fern parses docs as MDX, which rejects HTML comments (`<\!-- -->`).
recipe-health.md is in the Fern nav (docs/index.yml) and carried the
Apache license header plus BEGIN/END AICR-HEALTH splice markers as HTML
comments, breaking `Publish Fern Docs`. Convert all three to MDX comment
syntax `{/* ... */}` and update the make recipe-health-docs splice
guard/awk to match (fixed-string grep + index() to avoid regex-escaping
the brace/star metacharacters). Splice remains idempotent.
Also update the marker-presence test (tools/health) and ADR-009 to the
new MDX markers.
ed12037 to
327e927
Compare
ArangoGutierrez
left a comment
There was a problem hiding this comment.
Reviewed at 327e927 — clean fix, no objection.
HTML-comment to MDX-comment conversion so the Fern publish job can parse recipe-health.md, with the matching Makefile splice update and the tools/health test. The grep -qF / awk index($0, ...) switch to fixed-string matching is the right call, since the new markers contain regex metacharacters ({, *, /, }).
Confirmed the marker migration is complete: a repo-wide search shows only four files reference the AICR-HEALTH markers and all four are updated here, and recipe-health-check does not grep the markers (so it is unaffected). Fern Check is green.
No impact to the release artifacts — the only Go change is the expected-marker strings in tools/health's test, and tools/health is a go run docs-generation tool, not part of the shipped binaries or images.
This approval satisfies the aicr-maintainer code-owner review for the maintainer-owned paths here (Makefile, tools/health/**). The docs/** files are under aicr-write and still need an approval from that team (not the author). The merge gate will also hold until the in-flight CI checks finish green.
Summary
Convert the HTML comments in
docs/user/recipe-health.mdto MDX comment syntax so thePublish Fern Docsworkflow can parse the page.Motivation / Context
Fern parses docs as MDX, which rejects HTML comments (
<\!-- -->).recipe-health.mdis in the Fern nav (docs/index.yml) and carried the Apache license header plus theBEGIN/END AICR-HEALTHsplice markers as HTML comments, breaking the publish job:(
container-images.mdhas the same license header but is not in the nav, so it is never MDX-parsed — only this file broke.)Fixes: N/A
Related: https://github.com/NVIDIA/aicr/actions/runs/27553892737/job/81447404791
Type of Change
Component(s) Affected
docs/,examples/)Makefile(recipe-health-docssplice target)Implementation Notes
docs/user/recipe-health.md: license header + bothAICR-HEALTHsplice markers converted from<\!-- ... -->to{/* ... */}.Makefile(recipe-health-docs): splice guard now usesgrep -qFand the awk splice usesindex($0, ...)(fixed-string match) to avoid regex-escaping the{,*,/,}metacharacters in the new markers.Testing
Risk Assessment
Rollout notes: N/A
Checklist
make testwith-race) — N/A, no Go source changedmake lint)git commit -S)