Skip to content

feat: normalize all changed reference docs after generation - #732

Open
Iheanacho-ai wants to merge 1 commit into
siderolabs:mainfrom
Iheanacho-ai:proper-doc-normalize
Open

feat: normalize all changed reference docs after generation#732
Iheanacho-ai wants to merge 1 commit into
siderolabs:mainfrom
Iheanacho-ai:proper-doc-normalize

Conversation

@Iheanacho-ai

Copy link
Copy Markdown
Member

What

Make mdx-normalize safe to run across the whole reference tree, and wire it
into every reference generator so generated docs are normalized automatically.

Why

mdx-normalize was hardcoded to two Omni files with always-on inline escaping, which corrupted real HTML (<table>\<table>), re-escaped safe {"<"}/{/* */}, and fenced nested lists. So it couldn't run on the Talos reference or twice, and only omni-cli/image-factory generators normalized at all.

Change

  • Tool: inline </{ escaping is now opt-in (--escape-inline); existing MDX expressions/comments are preserved; nested lists are re-indented, not fenced; accepts multiple file paths; every transformation is idempotent. Tests added for tables, expressions/comments, nested lists, and normalize-twice == normalize-once.
  • Makefile: normalize-doc / normalize-doc-local normalize the changed reference files (.mdx under any /reference/ path), no arguments needed, with flags chosen by path. Added the normalize step to the four generators that lacked it, so all eight now normalize their output.

Note: the container target runs the published mdx-normalize:latest image — CI must republish it for the container path; until then use -local.

Testing

  • go test ./... and go vet ./... pass.
  • make generate-talos-reference (v1.13): completion blocks fenced, all tables intact, no \</\{ anywhere.
  • Idempotent (second pass is a no-op); clean tree reports "no changed reference .mdx files".
  • Verified --escape-inline routing: bare <machine-id> escaped in omni cli.mdx; the 46-table omni-configuration.mdx normalizes to a no-op.
  • By: me and Claude Code.

@github-project-automation github-project-automation Bot moved this to To Do in Planning Sep 2, 2026
@talos-bot talos-bot moved this from To Do to In Review in Planning Sep 2, 2026
@smira smira removed this from Planning Sep 2, 2026
@Iheanacho-ai
Iheanacho-ai force-pushed the proper-doc-normalize branch 6 times, most recently from 72bfdf0 to 01bfdad Compare September 2, 2026 16:36
@sterlingkoch

Copy link
Copy Markdown
Member

Review pass, tooling findings. One blocking item, the rest are latent cases that aren't in the corpus today.

1. Blocking: the container path passes a flag the published image doesn't have. Makefile:317 and :323 send --escape-inline to ghcr.io/siderolabs/mdx-normalize:latest, which is built from main and rejects it. I built the base version to check: flag provided but not defined: -escape-inline, exit 2. Affects generate-omni-cli-reference, generate-omni-image-factory-reference, and the generate-omni-reference aggregate.

This is worse than a merge-window race. pull_if_missing at Makefile:266 skips the pull whenever a :latest image is already cached locally, so anyone with the old image stays broken after CI republishes until they manually docker pull. Pin to a digest bumped in the same PR, or force a pull for this one image.

2. Nothing in CI runs the tests. tools/mdx-normalize/main_test.go:209. TestIdempotent is the guarantee this whole PR rests on, and make code-review is golangci-lint plus hadolint plus checkmake only. docs-ci.yaml doesn't touch the tool. Worth adding a go test ./... step over the tool modules so the guarantee has a gate.

3. An unpaired backtick silently disables escaping for the rest of the line. tools/mdx-normalize/main.go:150-154. Verified with this input, which comes back unchanged and ships a bare <machine-id>, the build error the flag exists to prevent:

use `omnictl and then <machine-id> is required

No such line in the corpus today, but this is upstream CLI help text we don't control. Consider only honoring the toggle when the line has an even backtick count.

4. The one true non-idempotency I could find. tools/mdx-normalize/main.go:227-231. A tab-indented fence under a colon intro. fenceRe at :60 only allows leading spaces, so a fence indented with a tab falls into the tab-block branch and gets wrapped again, and pass 2 then escapes inside the mangled output. Not in any file today. Cheap guard: bail out of the fence case when the block already contains a fence line.

5. CRLF defeats the whole pass. tools/mdx-normalize/main.go:118-120 trims only spaces and tabs, and blankRe / colonRe / hrRe at :61-63 are anchored with no \r?. On a CRLF file frontmatter isn't detected, so --escape-inline rewrites description: against the "always preserved verbatim" claim at :37, \r reads as a non-blank line so intro is lost, and a tab-indented command example de-indents into bare prose. Nothing emits CRLF today; the fix is small.

6. Output isn't regenerated in this PR. The next make generate-talos-reference will produce a 12-line diff in public/talos/v1.14/reference/cli.mdx that isn't here. It's a real fix (tab sub-bullets under "For each node:"), just simpler to commit it in this PR than to leave it as unrelated churn in someone else's.

Two nits. Makefile:320 and :333 call the tool once per file, so generate-talos-reference starts 93 containers even though the tool now accepts multiple paths. And $$roots / $$files are word-split unquoted at :301, :320, :333, which only matters for a path with a space.

Verified, and this held up well. I built the tool and ran it twice over all 288 reference .mdx files in the repo: pass 2 is byte-identical to pass 1. All eight adversarial cases are idempotent under both flag settings (table with < in a cell, pre-indented nested list, fence containing a fake nested list, {/* */} inside a code block, multi-line JSX, <foo> and {bar} on one line, CRLF, no trailing newline). The escaping flip loses nothing: old always-on output is byte-identical to new --escape-inline output on both affected Omni files. Zero paths and a nonexistent path both exit 0 cleanly. List nesting checked against marked rather than reasoned about, where 2-space-per-level is correct for - parents through four levels. -i rather than -it on docker run is the right call for CI, and the normalize_flags single-source mapping is nice.

@Iheanacho-ai
Iheanacho-ai force-pushed the proper-doc-normalize branch 3 times, most recently from 70794fd to 6624822 Compare September 4, 2026 14:31
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.

4 participants