build(docs): add documentation check and reference generation scripts - #2371
Conversation
4c3b972 to
bbcf725
Compare
bbcf725 to
6b02535
Compare
Rebased onto main, which renamed aks.draftArgoCDDeployment to "AKS: Create Argo CD Application" in Azure#2353. Regenerated so the committed reference matches package.json; otherwise docs:reference:check would fail once Azure#2371 lands.
825f41f to
38882af
Compare
Rebased onto main, which renamed aks.draftArgoCDDeployment to "AKS: Create Argo CD Application" in Azure#2353. Regenerated so the committed reference matches package.json; otherwise docs:reference:check would fail once Azure#2371 lands.
Rebased onto main, which renamed aks.draftArgoCDDeployment to "AKS: Create Argo CD Application" in Azure#2353. Regenerated so the committed reference matches package.json; otherwise docs:reference:check would fail once Azure#2371 lands.
|
Checked this out and ran both scripts against the tree, plus probed the parsers directly. The direction is right — deriving menu paths from 1.
|
| Prose | Actual default path |
|---|---|
features/tcp-dumps.md:5 — Troubleshoot Network Health ... Collect TCP Dumps |
classic only; default is Troubleshoot & Diagnose > Troubleshoot Network Health > Collect TCP Dumps |
features/kaito-install-deploy.md:7,17 — Deploy an LLM with KAITO ... Install KAITO |
Develop & Deploy > Deploy a LLM with KAITO > Install KAITO (title differs too — "a" not "an") |
features/inspektor-gadget.md:11 — Show Inspektor Gadget ... Gadget Commands |
not present in either menu mode |
features/aks-diagnostics.md:5 — Run AKS Diagnostics |
classic only |
features/aks-compare-cluster.md:5 — "right click on your AKS cluster" ... Compare AKS Cluster |
lives on the subscription node |
These are the simplifiedMenuStructure regressions the PR description is about. Two ways to close it, either works: teach crumbsIn to treat "and then select / and choose / and click on" as separators, or normalise the prose to > in #2372 and note that here.
2. The checks dropped in 285c765 aren't covered by anything yet
The commit removes links / images / anchors / summary on the grounds that lychee and mdbook already do it. The reasoning is sound, but neither gate exists on PRs today:
lycheeappears nowhere in the repo — not in.github/workflows/, not indocs/book/Makefile.create-missing = falseis inbook.tomlonmain(thanks to docs: fix broken links, restore missing Development page, drop duplicate copies #2370), butwebsite.yamltriggers only onpushtomain, so it never runs on a PR.
Net effect is a window where link/image/anchor checking on PRs is covered by nothing. Landing the lychee workflow alongside this would close it.
3. docs:reference:check fails on a clean checkout
The generated pages aren't committed, so on a fresh clone:
$ node scripts/generate-docs-reference.js --check
[stale] docs/book/src/reference/commands.md
[stale] docs/book/src/reference/settings.md
[stale] docs/book/src/reference/pinned-versions.md
exit 1
They're also not in SUMMARY.md, so mdbook won't publish them. Assuming that's #2372 — worth stating in the description, since as-is the script ships unable to pass.
4. The submenu cycle guard doesn't guard
scripts/lib/menu-graph.js:
const key = `${bucket}|${breadcrumb.join(">")}`;
if (active.has(key)) return; // submenu cycle guardbreadcrumb grows on every hop, so the key is fresh each recursion and the guard never hits. Reproduced with an A→B→A submenu pair: unbounded recursion to stack overflow rather than a clean stop. Keying on bucket alone fixes it.
5. false / never in a view menu reads as reachable
evaluateWhen skips any conjunct matching no CONJUNCT_RULES row:
evaluateWhen("viewItem =~ /aks\.cluster/i && never", true)
=> [{ node: 'cluster', flags: [] }]
buildPaletteVisibility handles false / never for the palette, but the menu walk doesn't — so a command hidden that way would be documented as present in the menu, and a breadcrumb pointing at it would validate.
6. NODE_PATTERNS asymmetry, and a dead diagnostic
The cluster row requires =~ /aks\.cluster; the subscription row also accepts == via [=~]+. So:
nodeOf("view == kubernetes.cloudExplorer && viewItem == aks.cluster") => 'any'
which renders as Any > ... with nothing flagged. Relatedly, the orphanNode check in generate-docs-reference.js looks for paths starting with "Other", but nodeOf only ever returns cluster | subscription | fleet | k8s-cluster | azure | any — so NODE_LABEL.other and that diagnostic are both unreachable. Pointing it at "Any" would make it live.
7. Nothing in CI looks at this code
eslint.config.mjs ignores **/*.js, and format-check.yml globs ts,tsx,json,css,md and only triggers on PRs touching ts/tsx. All checks are green here, but none of them read the new 963 lines. Prettier passes today (I ran it), just not by any gate.
Given splitTop, stripOuterParens, normaliseBoolean and evaluateWhen are the load-bearing parts, a few unit tests would be worth it — items 4, 5 and 6 above are each about five lines of test away from being caught automatically.
Smaller things
process.exit()right afterconsole.log(docs-check.js,generate-docs-reference.js) can truncate piped stdout in CI.process.exitCodeis safer.contributions().versionis computed and never read.const pkg = readJson("package.json")ingenerate-docs-reference.jsis unused —menu-graphalready exportspkg.CLASSIC_MARKERis the only escape hatch and is documented only in the source. There's no general "this bold chain isn't a menu path" opt-out, so a future page describing portal navigation as**Settings** > **Storage**will false-positive, sinceStorageis a real command title.identifiersdoesn't skip fenced code blocks. Clean today, but asettings.jsonexample naming another extension'sazure.*setting becomes a hard error.- Branch is 9 commits behind
main(2.5.0, docs: fix broken links, restore missing Development page, drop duplicate copies #2370, Publish release 2.5.0 #2388, Re-enable release pipeline publishing #2389) — worth a rebase.
What's good
The incidental findings the generator prints are real and worth filing on their own:
dead submenus (declared, never referenced): aks.tcpDataCollectionSubMenu
duplicate palette title "Create KAITO Workspace": aks.aksKaitoGenerateYaml, aks.aksKaitoCreateCRD
title repeats its category, palette shows "AKS: AKS: Launch Kickstart Agent": aks.kickstart.launchExperience
title repeats its category, palette shows "AKS: AKS: Configure Kickstart Cluster": aks.kickstartCluster
version skew actions/checkout: v3 vs v7.0.0
version skew azure/login: v1.4.6 vs v3.0.0
version skew Azure/k8s-deploy: v4 vs v6.0.0
version skew azure/aks-set-context: v3 vs v5.0.0
version skew azure/use-kubelogin: v1 vs v1.3
normaliseBoolean collapsing == true / != false is a genuine fix, and the commit messages explaining the reversal in 285c765 are unusually clear.
Items 1–3 are the ones I'd want resolved before merge; the rest are small.
Tatsat (Tats) Mishra 🐉 (Tatsinnit)
left a comment
There was a problem hiding this comment.
Line-level notes to go with the summary above. Nothing new versus that comment — just pinned to the relevant lines so they're easier to action. Items on RIGHT_CLICK, the cycle guard, !rule, and NODE_PATTERNS are the substantive ones; the rest are cleanup.
There was a problem hiding this comment.
Pull request overview
Adds documentation tooling to mechanically validate docs against package.json and to generate “reference” pages (commands, settings, pinned versions) so the docs can’t silently drift from the extension’s contributed surface area.
Changes:
- Introduces
scripts/docs-check.jswith checks for identifiers, menu breadcrumb paths, documentation coverage, and orphaned images. - Adds
scripts/generate-docs-reference.jsto generate (and--check) reference markdown pages underdocs/book/src/reference/. - Adds a shared
scripts/lib/menu-graph.jsthat derives the cluster context menu structure frompackage.json, used by both scripts to ensure consistent interpretation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/lib/menu-graph.js | Builds a menu graph from package.json contributions and evaluates when clauses across menu modes. |
| scripts/generate-docs-reference.js | Generates (and checks for drift in) reference docs pages: commands, settings, pinned versions. |
| scripts/docs-check.js | Implements docs validations that require package.json knowledge (IDs, menu paths, coverage, orphans). |
| package.json | Adds docs:check, docs:reference, and docs:reference:check npm scripts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Adds npm run docs:check, docs:reference, and docs:reference:check, backed by dependency-free Node. An audit of docs/ found roughly 60 inaccuracies against 2.3.0, and about 70% were hand-copied restatements of package.json: command IDs, menu paths, setting defaults, pinned versions. Nothing ran before merge to catch them, since website.yaml only triggers on push to main. scripts/lib/menu-graph.js resolves the cluster context menu from package.json, evaluated once per value of aks.simplifiedMenuStructure. Both scripts share it so they cannot disagree about the menu layout. docs-check.js runs eight checks, selectable by name: links, images, anchors, summary, identifiers, menu-paths, coverage, orphans. Errors exit non-zero; warnings do not. generate-docs-reference.js renders the menu graph, settings, and pinned versions as markdown. --check fails on drift. Classification that is data rather than control flow is kept in ordered tables (NODE_PATTERNS, CONJUNCT_RULES, CONDITION_RULES), so supporting a new tree node, context key, or gating condition means adding a row rather than editing a branch chain. Against main this reports 6 real errors: SUMMARY.md links to a development.md that does not exist (so mdbook publishes that section blank), two links resolve outside the book, and two retina-capture.md menu paths describe the classic menu. All are fixed in the follow-up PRs and left visible here so the tooling can be reviewed on its own.
Markdown allows raw HTML, and aks-plugins-github-copilot.md uses <img> to control image size. linksOf only parsed  syntax, so those three images were reported as orphaned when they are referenced. Also parses <a href> for the link check. Caught by cross-checking against lychee, which reported 3 broken image links where docs-check reported none.
Removes the links, images, anchors, and summary checks, and the helpers only they used. lychee --offline --include-fragments covers links, images, and anchors, handles URL fragments and raw HTML, and is maintained. mdbook build with create-missing = false fails on a SUMMARY entry with no page. Both are intended as CI gates, so this file was duplicating them. Duplicating them was actively worse: the version here only parsed markdown  syntax, so three images referenced with <img> tags read as orphaned. lychee caught that; this file did not. What remains is the part nothing off the shelf can do, because it needs to understand package.json: identifiers command IDs in prose that package.json does not contribute menu-paths menu breadcrumbs that do not match the real menu coverage commands documented nowhere in prose (warning) orphans images no page references (warning) 365 lines, down from 462.
Three defects found reviewing the checks against real prose. `config.aks.simplifiedMenuStructure == true` was not recognised as the menu-mode switch. CONJUNCT_RULES matched only the bare and `!`-negated spellings, so the `== true` form fell through to the generic `^config\.` rule and was reported as an ordinary runtime flag. The command would then be listed under both menu columns and menu-paths would accept a breadcrumb from either layout. package.json already uses `== true` for aks.kickstartEnabledPreview, so this was one edit from being live. normaliseBoolean now collapses `key`, `!key`, `== true`, `!= false`, `== false` and `!= true`, and the generator's renderCondition uses it too rather than its own partial copy. rootOf scanned the whole line for "fleet" or "subscription", so ordinary sentences resolved to the wrong tree node: "Right-click your AKS cluster > **Manage Cluster** > **Delete Cluster**. This removes the fleet member too." -> fleet That is a false error against correct documentation. Node detection now reads only the text leading up to the breadcrumb, which is what the right-click regex already captured but discarded. The coverage check lowercased both sides and used a substring match, so "Storage", "Best Practices", "Node Health" and "Profile CPU" were satisfied by any prose containing those words. It reported commands as documented that appear nowhere. A title now counts only where it stands alone as a UI string: bold, code span, heading, or a list item naming just that command, with an optional trailing ellipsis since titles like "Sign in to Azure..." are written without it in prose. On the current docs this moves the warnings from 17 incidental matches to 17 real ones, with different membership. Also removed a second, stale header comment advertising `links` and `images` checks that were deliberately dropped in favour of lychee, and dropped the local NLS resolver in favour of loc() from the shared module. The two disagreed on a missing key: loc throws, the local copy returned the raw "%key%" and let it through.
Three defects in the menu graph, each found by review and each now covered by a test. The submenu cycle guard keyed on bucket plus breadcrumb. The breadcrumb grows on every hop, so the key was fresh on each recursion and the guard never fired: an A -> B -> A pair recursed until the stack overflowed rather than stopping. The check now keys on the bucket alone, and runs before the label is recorded rather than only before the recursion, so a cycle contributes no path instead of one dangling breadcrumb. evaluateWhen skipped any conjunct matching no CONJUNCT_RULES row, which left the disjunct satisfiable. `false` and `never` therefore read as reachable, so a command hidden that way would be documented as present in the menu and a breadcrumb pointing at it would validate. buildPaletteVisibility already handled both for the palette; the menu walk now does too. Only the subscription row of NODE_PATTERNS tolerated the equality form, so `viewItem == aks.cluster` fell through to "any" and rendered as `Any > ...` with nothing flagged. Every row now accepts both forms. Tests run on plain node and mocha with no compile step, via npm run test:scripts, and cover splitTop, stripOuterParens, normaliseBoolean and evaluateWhen alongside the three regressions above.
…ead code The unresolved-node diagnostic filtered on paths starting with "Other", but nodeOf only ever returns cluster, subscription, fleet, k8s-cluster, azure or any. The filter was always empty and NODE_LABEL.other unreachable. It now targets the label for "any", which is the value that actually signals an unresolved node, and would have surfaced the equality-form gap on its own. process.exit() immediately after console.log can discard buffered stdout when it is a pipe rather than a TTY, which is how CI runs these. Both scripts now set process.exitCode and return. The identifiers check no longer scans fenced code blocks. Samples are quoted from elsewhere: a settings.json snippet naming another extension's `azure.*` key is not a claim about what this extension contributes. Fences are kept so line numbers stay accurate, and inline code is left alone because the coverage check relies on it naming real commands. Also drops contributions().version, the pkg and readJson bindings in the generator, and the friendlyFlag alias, none of which had a reader.
package-scripts.md covers the npm scripts but was never updated when docs:check and the reference generator were added, so the only description of either lived in the source. The classic-menu opt-out mattered most: a docs author hitting a false positive on menu-paths had no way to discover the marker existed, or that it is meant for pages genuinely about the classic layout rather than for silencing a stale breadcrumb.
38882af to
7ce6213
Compare
menu-paths recognises a breadcrumb only when > separates the steps. The book
writes navigation as prose ("and select", "and then click on"), so 23 of the
right-click instructions naming a bold UI element were skipped rather than
validated. That failure was silent, which is the same shape of problem the
menu checks exist to prevent: retina-capture.md line 5 went unreported while
lines 11 and 33 of the same file were caught, all three describing the same
stale path.
Teaching the parser the connector phrases was tried and rejected. It takes
menu-paths from 2 errors to 18, but prose does not distinguish a menu hop
from a step inside a wizard, since both are written "and select". It read
`**Create Cluster** and select **Create Standard Cluster**` as a two-level
menu path when the second is a button in the dialog the first opens, and it
misdiagnosed three omitted parent levels as a wrong node. The phrase list
would never be complete either, so every unknown phrasing stayed silent.
menu-syntax instead checks the convention, not the menu, so it cannot make
that mistake. A line menu-paths can already read is exempt, so the two
retina-capture lines are not reported twice. Bold that precedes the
right-click is ignored, because naming a UI element is not an instruction,
and a page can opt out entirely with `docs-check: not-a-menu`.
Requiring the module no longer runs the checks, so the line-level parsers can
be tested directly. Doing that caught a wrong menu path in the example added
to the development page: it omitted Troubleshoot Network Health.
… a host
CodeQL flagged both withoutUrls assertions as
js/incomplete-url-substring-sanitization: `!out.includes("dev.azure.com")`
reads as a URL check that an attacker-controlled host could slip past. It is
a test rather than a sanitiser, but the assertion was weak for the same
reason the rule exists — it only proved the host was absent, not that the
URL had been blanked to the right width.
Now asserts exact output. Splits the link case onto a relative target as
well, since the URL rule consumes the trailing paren of a `](http...)` target
before the link rule sees it, which the previous single case obscured.
The marker search scanned the whole file, so the page documenting the markers matched its own examples and opted itself out. Matched against fence-blanked text instead; a real marker still works. [skip pr-size]
|
Will be adding a PR gate for lychee link checks in subsequent PR. |
f30f82a
into
Azure:main
PR 1 of 3. Tooling only, no documentation changes.
Why
An audit of
docs/found ~60 inaccuracies against2.3.0. About 70% were hand-copied restatements ofpackage.json: command IDs, menu paths, setting defaults, pinned versions. Nothing ran before merge to catch them, becausewebsite.yamlonly triggers on push tomain.aks.simplifiedMenuStructuredefaulting totrueinvalidated the "how to open this" instructions across 11 pages in one change. That is mechanically detectable, so it should be mechanically detected.What this adds
npm run docs:checkpackage.jsonnpm run docs:referencenpm run docs:reference:checknpm run test:scriptsscripts/lib/menu-graph.jsresolves the cluster context menu frompackage.json, evaluating eachwhenclause once per value ofaks.simplifiedMenuStructure. Both scripts share it, so they cannot disagree about the menu layout.docs-check.jsruns four checks, each selectable by name (npm run docs:check menu-paths):identifierspackage.jsondoes not contributemenu-pathsmenu-syntax**A** > **B**coverageorphansIt deliberately does not check links, images, anchors, or SUMMARY completeness.
lychee --offline --include-fragmentscovers the first three and handles URL fragments and raw HTML properly.mdbook buildwithcreate-missing = falsefails on a SUMMARY entry with no page. Both are intended as CI gates, so duplicating them here was redundant, and measurably worse: an earlier version of this file missed images referenced with<img>tags, which lychee caught.generate-docs-reference.jsrenders the menu graph, settings, and pinned third-party versions as markdown.--checkregenerates in memory and fails on drift, so committed output cannot silently fall behindpackage.json.Review fixes
Three defects in the menu graph, each now covered by a test:
falseandneverread as reachable.evaluateWhenskipped any conjunct matching no rule, leaving the disjunct satisfiable, so a command hidden that way would be documented as present in the menu and a breadcrumb pointing at it would validate.buildPaletteVisibilityalready handled both for the palette; the menu walk now does too.viewItem == aks.clusterfell through toanyand rendered asAny > ...with nothing flagged. EveryNODE_PATTERNSrow now accepts both=~and==.!=is deliberately excluded, since it would resolve a negation to the node it excludes.Also addressed:
"Other", whichnodeOfnever returns, so it was always empty andNODE_LABEL.otherunreachable. It now targets the label forany, which is the value that actually signals an unresolved node, and would have caught the equality-form gap on its own.process.exit()immediately afterconsole.logcan discard buffered stdout when it is a pipe rather than a TTY, which is how CI runs these. Both scripts now setprocess.exitCodeand return.identifiersno longer scans fenced code blocks, so asettings.jsonsample naming another extension'sazure.*key is not a hard error. Fences are kept so line numbers stay accurate; inline code is untouched, becausecoveragerelies on it naming real commands.contributions().version, thepkgandreadJsonbindings in the generator, and thefriendlyFlagalias, none of which had a reader.classic-menuopt-out was documented only in the source, so a docs author hitting a false positive had no way to discover it. Documented on the Package Scripts page along with the three docs scripts, which were never added there when they were introduced.Rebased onto
main. Worth flagging for anyone reviewing the diff:docs/package-scripts.mdwas deleted in #2370 as a stale fork, so that documentation went todocs/book/src/development/development.mdinstead.Testing
npm run test:scripts— 53 tests onsplitTop,stripOuterParens,normaliseBoolean,evaluateWhen,walk,crumbsIn,proseNavIn,withoutFencedCodeandwithoutUrls, including a regression for each of the three defects above. The cycle test builds a synthetic A → B → A submenu pair and asserts the walk terminates; against the previous guard it raisesRangeError: Maximum call stack size exceeded.Nothing in CI runs them yet.
eslint.config.mjsignores**/*.jsandformat-check.ymlonly triggers onts/tsx, sotest:scripts,docs:checkand Prettier overscripts/are all green locally and unenforced. Wiring that up is follow-up work, together with the lychee gate.menu-pathswas verified by deliberately breaking things: renaming a submenu inpackage.nls.jsonflags every affected breadcrumb with the correct new path, and moving a command to another tree node is reported as such.--checkwas verified to exit 1 on drift and 0 after regeneration.Current state against
mainnpm run docs:checkreports 2 errors: tworetina-capture.mdmenu paths describe the classic menu rather than the grouped default. Fixed in #2372, left visible here so this can be reviewed as tooling on its own.npm run docs:reference:checkfails on a clean checkout, because the generated pages underdocs/book/src/reference/are not committed and are not inSUMMARY.md. Both land in #2372, which keeps this PR to tooling.The
>convention, andmenu-syntaxmenu-pathsrecognises a breadcrumb only when>separates the steps. The book writes navigation as prose ("and select", "and then click on"), so 23 of the right-click instructions naming a bold UI element were skipped rather than validated.That failure was silent, which is the same shape of problem these checks exist to prevent. It is why
retina-capture.md:5went unreported while lines 11 and 33 of the same file were caught, despite all three describing the same stale path.Why not teach the parser the connector phrases
Prototyped and rejected. It works, and takes
menu-pathsfrom 2 errors to 18, including all five cases raised in review. But the 16 new ones are not uniform:image-cleaner-eraser-tool.md:5reports "on the k8s-cluster node, not cluster" when it is on the cluster node, atTroubleshoot & Diagnose > Run Eraser Image Cleanup. The real fault is an omitted parent level, so the message points at the wrong fix.show-properties-azureportal-start-stop.md:25writes**Create Cluster** and select **Create Standard Cluster**, where the second is a button in the wizard the first opens.The false positive is not fixable with a better phrase list. Prose uses the same connector for a menu hop and an in-app action, so the parser has no signal to separate them, and the list would never be complete either.
What this does instead
menu-syntaxkeeps>required and makes omitting it a visible error rather than silence. It checks the convention, not the menu, so it cannot produce the wizard-step mistake.>also states where the menu ends, which prose cannot, so it helps readers for the same reason it helps the checker.A line
menu-pathscan already read is exempt, so nothing is reported twice. Bold preceding the right-click is ignored, since naming a UI element is not an instruction. A page can opt out withdocs-check: not-a-menu.23 errors today; #2372 does the rewrites. Worth flagging for that PR: it is more than punctuation, since several pages describe commands as sitting directly on the cluster menu when they are nested. The real paths should come from the generated
reference/commands.md.Requiring
docs-check.jsas a module no longer runs the checks, so the line-level parsers are testable directly. Doing that immediately caught a wrong menu path in the example added to the development page, which had omittedTroubleshoot Network Health.Follow-up work
mdbook build,docs:check,docs:reference:check,test:scripts, and Prettier/ESLint coverage forscripts/aks.tcpDataCollectionSubMenu, duplicate palette title "Create KAITO Workspace",AKS: AKS:double-prefix on two kickstart commands, and five GitHub Actions version skews